[PATCH 0/4] some fs/tftp updates

[PATCH 0/4] some fs/tftp updates

[Sascha Hauer]

This series has some updates for the FS and TFTP code

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (4):
      fs: open loopback device before using it
      fs: tftp: cache dentries for some time
      fs: kill invalid dentries
      fs: tftp: Set default window size to 1

 arch/arm/configs/imx_v7_defconfig   |  2 +-
 arch/arm/configs/multi_v7_defconfig |  2 +-
 fs/Kconfig                          |  2 +-
 fs/fs.c                             |  8 +++++
 fs/tftp.c                           | 58 +++++++++++++++++++++++++++++++++++--
 5 files changed, 67 insertions(+), 5 deletions(-)
---
base-commit: 822f814ffd12a52b998df4017ca13553709853b2
change-id: 20241107-fs-tftp-a5a90de2857b

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/4] fs: open loopback device before using it

[Sascha Hauer]

The loopback cdev created with cdev_create_loop() is used, but never
opened. Open it before usage.

Not having it opened has the effect that during unmount of a loopback
mounted file the devfs_remove() call for the loopback cdev fails,
because the usage counter drops below zero.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 5e7e05f499..f24962d2a6 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -933,6 +933,13 @@ int fsdev_open_cdev(struct fs_device *fsdev)
 			return ret;
 
 		fsdev->cdev = cdev_create_loop(fsdev->backingstore, O_RDWR, offset);
+		if (fsdev->cdev) {
+			ret = cdev_open(fsdev->cdev, O_RDWR);
+			if (ret) {
+				cdev_remove_loop(fsdev->cdev);
+				fsdev->cdev = NULL;
+			}
+		}
 	} else {
 		fsdev->cdev = cdev_open_by_name(fsdev->backingstore, O_RDWR);
 	}

-- 
2.39.5

[PATCH 2/4] fs: tftp: cache dentries for some time

[Sascha Hauer]

For TFTP we always returned invalid in the d_revalidate op. This has
the effect that we always look on the server if a file still exists, but
has also the effect that we initiate multiple tftp connections when we
want to transfer a file.

This patch introduces a grace time of two seconds in which a dentry will
be valid before looking it up on the server again. Two seconds is long
enough for not initiating multiple TFTP connections when transferring a
file and also short enough that changes on the server will be recognized
by barebox.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/tftp.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index c6edc9969f..cc9c4ab1f7 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -130,6 +130,16 @@ struct tftp_priv {
 	IPaddr_t server;
 };
 
+struct tftp_inode {
+	struct inode inode;
+	u64 time;
+};
+
+static struct tftp_inode *to_tftp_inode(struct inode *inode)
+{
+	return container_of(inode, struct tftp_inode, inode);
+}
+
 static inline bool is_block_before(uint16_t a, uint16_t b)
 {
 	return (int16_t)(b - a) > 0;
@@ -939,10 +949,14 @@ static struct inode *tftp_get_inode(struct super_block *sb, const struct inode *
                                      umode_t mode)
 {
 	struct inode *inode = new_inode(sb);
+	struct tftp_inode *node;
 
 	if (!inode)
 		return NULL;
 
+	node = to_tftp_inode(inode);
+	node->time = get_time_ns();
+
 	inode->i_ino = get_next_ino();
 	inode->i_mode = mode;
 
@@ -1015,7 +1029,47 @@ static const struct inode_operations tftp_dir_inode_operations =
 	.create = tftp_create,
 };
 
-static const struct super_operations tftp_ops;
+static struct inode *tftp_alloc_inode(struct super_block *sb)
+{
+	struct tftp_inode *node;
+
+	node = xzalloc(sizeof(*node));
+	if (!node)
+		return NULL;
+
+	return &node->inode;
+}
+
+static void tftp_destroy_inode(struct inode *inode)
+{
+	struct tftp_inode *node = to_tftp_inode(inode);
+
+	free(node);
+}
+
+static const struct super_operations tftp_ops = {
+	.alloc_inode = tftp_alloc_inode,
+	.destroy_inode = tftp_destroy_inode,
+};
+
+static int tftp_lookup_revalidate(struct dentry *dentry, unsigned int flags)
+{
+	struct tftp_inode *node;
+
+	if (!dentry->d_inode)
+		return 0;
+
+	node = to_tftp_inode(dentry->d_inode);
+
+	if (is_timeout(node->time, 2 * SECOND))
+		return 0;
+
+	return 1;
+}
+
+static const struct dentry_operations tftp_dentry_operations = {
+	.d_revalidate = tftp_lookup_revalidate,
+};
 
 static int tftp_probe(struct device *dev)
 {
@@ -1034,7 +1088,7 @@ static int tftp_probe(struct device *dev)
 	}
 
 	sb->s_op = &tftp_ops;
-	sb->s_d_op = &no_revalidate_d_ops;
+	sb->s_d_op = &tftp_dentry_operations;
 
 	inode = tftp_get_inode(sb, NULL, S_IFDIR);
 	sb->s_root = d_make_root(inode);

-- 
2.39.5

[PATCH 3/4] fs: kill invalid dentries

[Sascha Hauer]

When a dentry becomes invalid we can safely kill it and free the
associated memory. This fixes a memory leakage in the tftp and nfs
code as these regularly declare a dentry as invalid.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fs.c b/fs/fs.c
index f24962d2a6..a9d2cc7088 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1577,6 +1577,7 @@ static struct dentry *lookup_dcache(const struct qstr *name,
 			if (!error)
 				d_invalidate(dentry);
 			dput(dentry);
+			dentry_kill(dentry);
 			return ERR_PTR(error);
 		}
 	}

-- 
2.39.5

[PATCH 4/4] fs: tftp: Set default window size to 1

[Sascha Hauer]

The TFTP window size feature is nice when it works, unfortunately it
often causes trouble as often packages get lost in the network buffers
causing retransmission delays. Disable this option by default.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/configs/imx_v7_defconfig   | 2 +-
 arch/arm/configs/multi_v7_defconfig | 2 +-
 fs/Kconfig                          | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index d3def0ece0..72833eb09f 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -225,7 +225,7 @@ CONFIG_SYSCON_REBOOT_MODE=y
 CONFIG_POWER_RESET_SYSCON=y
 CONFIG_FS_EXT4=y
 CONFIG_FS_TFTP=y
-CONFIG_FS_TFTP_MAX_WINDOW_SIZE=8
+CONFIG_FS_TFTP_MAX_WINDOW_SIZE=1
 CONFIG_FS_NFS=y
 CONFIG_FS_FAT=y
 CONFIG_FS_FAT_WRITE=y
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 97b01fd812..5b739a0f3b 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -344,7 +344,7 @@ CONFIG_VIRTIO_MMIO=y
 CONFIG_FS_CRAMFS=y
 CONFIG_FS_EXT4=y
 CONFIG_FS_TFTP=y
-CONFIG_FS_TFTP_MAX_WINDOW_SIZE=8
+CONFIG_FS_TFTP_MAX_WINDOW_SIZE=1
 CONFIG_FS_NFS=y
 CONFIG_FS_FAT=y
 CONFIG_FS_FAT_WRITE=y
diff --git a/fs/Kconfig b/fs/Kconfig
index e02cb939a5..27981b6444 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -47,7 +47,7 @@ config FS_TFTP_MAX_WINDOW_SIZE
 	int
 	prompt "maximum tftp window size (RFC 7440)"
 	depends on FS_TFTP
-	default 128
+	default 1
 	range 1 128
 	help
 	  The maximum allowed tftp "windowsize" (RFC 7440).  Higher

-- 
2.39.5

Re: [PATCH 0/4] some fs/tftp updates

[Sascha Hauer]

On Thu, 07 Nov 2024 08:23:29 +0100, Sascha Hauer wrote:
> This series has some updates for the FS and TFTP code
> 
> 

Applied, thanks!

[1/4] fs: open loopback device before using it
      https://git.pengutronix.de/cgit/barebox/commit/?id=89ecaaf3bb39 (link may not be stable)
[2/4] fs: tftp: cache dentries for some time
      https://git.pengutronix.de/cgit/barebox/commit/?id=2ac80263b1f1 (link may not be stable)
[3/4] fs: kill invalid dentries
      https://git.pengutronix.de/cgit/barebox/commit/?id=840818621fba (link may not be stable)
[4/4] fs: tftp: Set default window size to 1
      https://git.pengutronix.de/cgit/barebox/commit/?id=5d943ab80e83 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>