netfs.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short
@ 2025-04-28 15:48 Max Kellermann
  2025-04-28 15:48 ` [PATCH 2/4] fs/netfs: reorderd struct fields to eliminate holes Max Kellermann
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Max Kellermann @ 2025-04-28 15:48 UTC (permalink / raw)
  To: dhowells, netfs, linux-kernel; +Cc: Max Kellermann

The `error` field only needs to be able to hold an errno integer, and
a `short` is enough for that - just like in `struct
netfs_io_subrequest`.

This shrinks the struct from 608 to 600 bytes.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/netfs/main.c          | 2 +-
 fs/netfs/write_collect.c | 2 +-
 include/linux/netfs.h    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/netfs/main.c b/fs/netfs/main.c
index 70ecc8f5f210..fbb605ee0b28 100644
--- a/fs/netfs/main.c
+++ b/fs/netfs/main.c
@@ -65,7 +65,7 @@ static int netfs_requests_seq_show(struct seq_file *m, void *v)
 
 	rreq = list_entry(v, struct netfs_io_request, proc_link);
 	seq_printf(m,
-		   "%08x %s %3d %2lx %4ld %3d @%04llx %llx/%llx",
+		   "%08x %s %3d %2lx %4d %3d @%04llx %llx/%llx",
 		   rreq->debug_id,
 		   netfs_origins[rreq->origin],
 		   refcount_read(&rreq->ref),
diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index 3fca59e6475d..b405229de787 100644
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -23,7 +23,7 @@
 
 static void netfs_dump_request(const struct netfs_io_request *rreq)
 {
-	pr_err("Request R=%08x r=%d fl=%lx or=%x e=%ld\n",
+	pr_err("Request R=%08x r=%d fl=%lx or=%x e=%d\n",
 	       rreq->debug_id, refcount_read(&rreq->ref), rreq->flags,
 	       rreq->origin, rreq->error);
 	pr_err("  st=%llx tsl=%zx/%llx/%llx\n",
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index c86a11cfc4a3..da0d36615bef 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -253,7 +253,7 @@ struct netfs_io_request {
 	unsigned long long	submitted;	/* Amount submitted for I/O so far */
 	unsigned long long	len;		/* Length of the request */
 	size_t			transferred;	/* Amount to be indicated as transferred */
-	long			error;		/* 0 or error that occurred */
+	short			error;		/* 0 or error that occurred */
 	enum netfs_io_origin	origin;		/* Origin of the request */
 	bool			direct_bv_unpin; /* T if direct_bv[] must be unpinned */
 	unsigned long long	i_size;		/* Size of the file */
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] fs/netfs: reorderd struct fields to eliminate holes
  2025-04-28 15:48 [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short Max Kellermann
@ 2025-04-28 15:48 ` Max Kellermann
  2025-04-28 15:48 ` [PATCH 3/4] fs/netfs: remove `netfs_io_request.ractl` Max Kellermann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Max Kellermann @ 2025-04-28 15:48 UTC (permalink / raw)
  To: dhowells, netfs, linux-kernel; +Cc: Max Kellermann

This shrinks `struct netfs_io_stream` from 104 to 96 bytes and `struct
netfs_io_request` from 600 to 576 bytes.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 include/linux/netfs.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index da0d36615bef..f0436bac5b59 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -146,8 +146,8 @@ struct netfs_io_stream {
 	struct netfs_io_subrequest *front;	/* Op being collected */
 	unsigned long long	collected_to;	/* Position we've collected results to */
 	size_t			transferred;	/* The amount transferred from this stream */
-	enum netfs_io_source	source;		/* Where to read from/write to */
 	unsigned short		error;		/* Aggregate error for the stream */
+	enum netfs_io_source	source;		/* Where to read from/write to */
 	unsigned char		stream_nr;	/* Index of stream in parent table */
 	bool			avail;		/* T if stream is available */
 	bool			active;		/* T if stream is active */
@@ -243,19 +243,9 @@ struct netfs_io_request {
 	void			*netfs_priv;	/* Private data for the netfs */
 	void			*netfs_priv2;	/* Private data for the netfs */
 	struct bio_vec		*direct_bv;	/* DIO buffer list (when handling iovec-iter) */
-	unsigned int		direct_bv_count; /* Number of elements in direct_bv[] */
-	unsigned int		debug_id;
-	unsigned int		rsize;		/* Maximum read size (0 for none) */
-	unsigned int		wsize;		/* Maximum write size (0 for none) */
-	atomic_t		subreq_counter;	/* Next subreq->debug_index */
-	unsigned int		nr_group_rel;	/* Number of refs to release on ->group */
-	spinlock_t		lock;		/* Lock for queuing subreqs */
 	unsigned long long	submitted;	/* Amount submitted for I/O so far */
 	unsigned long long	len;		/* Length of the request */
 	size_t			transferred;	/* Amount to be indicated as transferred */
-	short			error;		/* 0 or error that occurred */
-	enum netfs_io_origin	origin;		/* Origin of the request */
-	bool			direct_bv_unpin; /* T if direct_bv[] must be unpinned */
 	unsigned long long	i_size;		/* Size of the file */
 	unsigned long long	start;		/* Start position */
 	atomic64_t		issued_to;	/* Write issuer folio cursor */
@@ -263,7 +253,17 @@ struct netfs_io_request {
 	unsigned long long	cleaned_to;	/* Position we've cleaned folios to */
 	unsigned long long	abandon_to;	/* Position to abandon folios to */
 	pgoff_t			no_unlock_folio; /* Don't unlock this folio after read */
+	unsigned int		direct_bv_count; /* Number of elements in direct_bv[] */
+	unsigned int		debug_id;
+	unsigned int		rsize;		/* Maximum read size (0 for none) */
+	unsigned int		wsize;		/* Maximum write size (0 for none) */
+	atomic_t		subreq_counter;	/* Next subreq->debug_index */
+	unsigned int		nr_group_rel;	/* Number of refs to release on ->group */
+	spinlock_t		lock;		/* Lock for queuing subreqs */
+	short			error;		/* 0 or error that occurred */
 	unsigned char		front_folio_order; /* Order (size) of front folio */
+	enum netfs_io_origin	origin;		/* Origin of the request */
+	bool			direct_bv_unpin; /* T if direct_bv[] must be unpinned */
 	refcount_t		ref;
 	unsigned long		flags;
 #define NETFS_RREQ_OFFLOAD_COLLECTION	0	/* Offload collection to workqueue */
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] fs/netfs: remove `netfs_io_request.ractl`
  2025-04-28 15:48 [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short Max Kellermann
  2025-04-28 15:48 ` [PATCH 2/4] fs/netfs: reorderd struct fields to eliminate holes Max Kellermann
@ 2025-04-28 15:48 ` Max Kellermann
  2025-04-28 15:48 ` [PATCH 4/4] fs/netfs: declare field `proc_link` only if CONFIG_PROC_FS=y Max Kellermann
  2025-05-07  9:01 ` [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: Max Kellermann @ 2025-04-28 15:48 UTC (permalink / raw)
  To: dhowells, netfs, linux-kernel; +Cc: Max Kellermann

Since this field is only used by netfs_prepare_read_iterator() when
called by netfs_readahead(), we can simply pass it as parameter.  This
shrinks the struct from 576 to 568 bytes.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 fs/netfs/buffered_read.c | 24 ++++++++++++------------
 include/linux/netfs.h    |  1 -
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 0d1b6d35ff3b..5f53634a3862 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -78,7 +78,8 @@ static int netfs_begin_cache_read(struct netfs_io_request *rreq, struct netfs_in
  * [!] NOTE: This must be run in the same thread as ->issue_read() was called
  * in as we access the readahead_control struct.
  */
-static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq)
+static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq,
+					   struct readahead_control *ractl)
 {
 	struct netfs_io_request *rreq = subreq->rreq;
 	size_t rsize = subreq->len;
@@ -86,7 +87,7 @@ static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq)
 	if (subreq->source == NETFS_DOWNLOAD_FROM_SERVER)
 		rsize = umin(rsize, rreq->io_streams[0].sreq_max_len);
 
-	if (rreq->ractl) {
+	if (ractl) {
 		/* If we don't have sufficient folios in the rolling buffer,
 		 * extract a folioq's worth from the readahead region at a time
 		 * into the buffer.  Note that this acquires a ref on each page
@@ -99,7 +100,7 @@ static ssize_t netfs_prepare_read_iterator(struct netfs_io_subrequest *subreq)
 		while (rreq->submitted < subreq->start + rsize) {
 			ssize_t added;
 
-			added = rolling_buffer_load_from_ra(&rreq->buffer, rreq->ractl,
+			added = rolling_buffer_load_from_ra(&rreq->buffer, ractl,
 							    &put_batch);
 			if (added < 0)
 				return added;
@@ -211,7 +212,8 @@ static void netfs_issue_read(struct netfs_io_request *rreq,
  * slicing up the region to be read according to available cache blocks and
  * network rsize.
  */
-static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
+static void netfs_read_to_pagecache(struct netfs_io_request *rreq,
+				    struct readahead_control *ractl)
 {
 	struct netfs_inode *ictx = netfs_inode(rreq->inode);
 	unsigned long long start = rreq->start;
@@ -291,7 +293,7 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq)
 		break;
 
 	issue:
-		slice = netfs_prepare_read_iterator(subreq);
+		slice = netfs_prepare_read_iterator(subreq, ractl);
 		if (slice < 0) {
 			ret = slice;
 			subreq->error = ret;
@@ -359,11 +361,10 @@ void netfs_readahead(struct readahead_control *ractl)
 
 	netfs_rreq_expand(rreq, ractl);
 
-	rreq->ractl = ractl;
 	rreq->submitted = rreq->start;
 	if (rolling_buffer_init(&rreq->buffer, rreq->debug_id, ITER_DEST) < 0)
 		goto cleanup_free;
-	netfs_read_to_pagecache(rreq);
+	netfs_read_to_pagecache(rreq, ractl);
 
 	netfs_put_request(rreq, true, netfs_rreq_trace_put_return);
 	return;
@@ -389,7 +390,6 @@ static int netfs_create_singular_buffer(struct netfs_io_request *rreq, struct fo
 	if (added < 0)
 		return added;
 	rreq->submitted = rreq->start + added;
-	rreq->ractl = (struct readahead_control *)1UL;
 	return 0;
 }
 
@@ -459,7 +459,7 @@ static int netfs_read_gaps(struct file *file, struct folio *folio)
 	iov_iter_bvec(&rreq->buffer.iter, ITER_DEST, bvec, i, rreq->len);
 	rreq->submitted = rreq->start + flen;
 
-	netfs_read_to_pagecache(rreq);
+	netfs_read_to_pagecache(rreq, NULL);
 
 	if (sink)
 		folio_put(sink);
@@ -528,7 +528,7 @@ int netfs_read_folio(struct file *file, struct folio *folio)
 	if (ret < 0)
 		goto discard;
 
-	netfs_read_to_pagecache(rreq);
+	netfs_read_to_pagecache(rreq, NULL);
 	ret = netfs_wait_for_read(rreq);
 	netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
 	return ret < 0 ? ret : 0;
@@ -685,7 +685,7 @@ int netfs_write_begin(struct netfs_inode *ctx,
 	if (ret < 0)
 		goto error_put;
 
-	netfs_read_to_pagecache(rreq);
+	netfs_read_to_pagecache(rreq, NULL);
 	ret = netfs_wait_for_read(rreq);
 	if (ret < 0)
 		goto error;
@@ -750,7 +750,7 @@ int netfs_prefetch_for_write(struct file *file, struct folio *folio,
 	if (ret < 0)
 		goto error_put;
 
-	netfs_read_to_pagecache(rreq);
+	netfs_read_to_pagecache(rreq, NULL);
 	ret = netfs_wait_for_read(rreq);
 	netfs_put_request(rreq, false, netfs_rreq_trace_put_return);
 	return ret < 0 ? ret : 0;
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index f0436bac5b59..547b1aa70d2a 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -231,7 +231,6 @@ struct netfs_io_request {
 	struct kiocb		*iocb;		/* AIO completion vector */
 	struct netfs_cache_resources cache_resources;
 	struct netfs_io_request	*copy_to_cache;	/* Request to write just-read data to the cache */
-	struct readahead_control *ractl;	/* Readahead descriptor */
 	struct list_head	proc_link;	/* Link in netfs_iorequests */
 	struct netfs_io_stream	io_streams[2];	/* Streams of parallel I/O operations */
 #define NR_IO_STREAMS 2 //wreq->nr_io_streams
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] fs/netfs: declare field `proc_link` only if CONFIG_PROC_FS=y
  2025-04-28 15:48 [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short Max Kellermann
  2025-04-28 15:48 ` [PATCH 2/4] fs/netfs: reorderd struct fields to eliminate holes Max Kellermann
  2025-04-28 15:48 ` [PATCH 3/4] fs/netfs: remove `netfs_io_request.ractl` Max Kellermann
@ 2025-04-28 15:48 ` Max Kellermann
  2025-05-07  9:01 ` [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: Max Kellermann @ 2025-04-28 15:48 UTC (permalink / raw)
  To: dhowells, netfs, linux-kernel; +Cc: Max Kellermann

This field is only used for the "proc" filesystem.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
 include/linux/netfs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 547b1aa70d2a..73b2a04aa801 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -231,7 +231,9 @@ struct netfs_io_request {
 	struct kiocb		*iocb;		/* AIO completion vector */
 	struct netfs_cache_resources cache_resources;
 	struct netfs_io_request	*copy_to_cache;	/* Request to write just-read data to the cache */
+#ifdef CONFIG_PROC_FS
 	struct list_head	proc_link;	/* Link in netfs_iorequests */
+#endif
 	struct netfs_io_stream	io_streams[2];	/* Streams of parallel I/O operations */
 #define NR_IO_STREAMS 2 //wreq->nr_io_streams
 	struct netfs_group	*group;		/* Writeback group being written back */
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short
  2025-04-28 15:48 [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short Max Kellermann
                   ` (2 preceding siblings ...)
  2025-04-28 15:48 ` [PATCH 4/4] fs/netfs: declare field `proc_link` only if CONFIG_PROC_FS=y Max Kellermann
@ 2025-05-07  9:01 ` kernel test robot
  2025-05-07 15:48   ` Max Kellermann
  3 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2025-05-07  9:01 UTC (permalink / raw)
  To: Max Kellermann, dhowells, netfs, linux-kernel
  Cc: oe-kbuild-all, Max Kellermann

Hi Max,

kernel test robot noticed the following build errors:

[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v6.15-rc5 next-20250506]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Max-Kellermann/fs-netfs-reorderd-struct-fields-to-eliminate-holes/20250428-235058
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250428154859.3228933-1-max.kellermann%40ionos.com
patch subject: [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short
config: sparc-randconfig-002-20250503 (https://download.01.org/0day-ci/archive/20250507/202505071602.yJrZsTiu-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250507/202505071602.yJrZsTiu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505071602.yJrZsTiu-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__cmpxchg_called_with_bad_pointer" [fs/netfs/netfs.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short
  2025-05-07  9:01 ` [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short kernel test robot
@ 2025-05-07 15:48   ` Max Kellermann
  0 siblings, 0 replies; 6+ messages in thread
From: Max Kellermann @ 2025-05-07 15:48 UTC (permalink / raw)
  To: dhowells, netfs, linux-kernel

On Wed, May 7, 2025 at 11:01 AM kernel test robot <lkp@intel.com> wrote:

> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
> >> ERROR: modpost: "__cmpxchg_called_with_bad_pointer" [fs/netfs/netfs.ko] undefined!

Okay, not all architectures support cmpxchg() with a short. I can drop
this patch, but I'm curious why netfs_read_to_pagecache() uses
cmpxchg() when everybody else simply assigns the value. Maybe, if
cmpxchg() turns out to be unnecessary, we can keep this patch? David,
can you explain? You added it in commit ee4cdf7ba857a ("netfs: Speed
up buffered reading").

Max

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-05-07 15:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-28 15:48 [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short Max Kellermann
2025-04-28 15:48 ` [PATCH 2/4] fs/netfs: reorderd struct fields to eliminate holes Max Kellermann
2025-04-28 15:48 ` [PATCH 3/4] fs/netfs: remove `netfs_io_request.ractl` Max Kellermann
2025-04-28 15:48 ` [PATCH 4/4] fs/netfs: declare field `proc_link` only if CONFIG_PROC_FS=y Max Kellermann
2025-05-07  9:01 ` [PATCH 1/4] fs/netfs: convert `netfs_io_request.error` to a `short kernel test robot
2025-05-07 15:48   ` Max Kellermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).