[PATCH] 9p: treat read return values of 0 as EOF

[PATCH] 9p: treat read return values of 0 as EOF

[Barret Rhoden]

If your 9p server shares a synthetic file system such as sysfs, the file
size from stat often disagrees with the actual file size.

Treat non-error reads of 0 as EOF, except in the case where the request
was for 0 bytes.

Tested:
	git@github.com:hugelgupf/p9.git, served "/" on 10.0.2.2

	bash-5.3# mount -t 9p -o trans=tcp,port=12345,cache=none 10.0.2.2 /tmp/foo
	bash-5.3# cat /tmp/foo/sys/devices/system/cpu/online
	0-47

Fixes: eb497943fa21 ("9p: Convert to using the netfs helper lib to do reads and caching")
Signed-off-by: Barret Rhoden <brho@google.com>
---
 fs/9p/vfs_addr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 1ac0b3dcc077..df420b061664 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -73,6 +73,7 @@ static void v9fs_issue_read(struct netfs_io_subrequest *subreq)
 	char *target;
 	unsigned long long pos = subreq->start + subreq->transferred;
 	int total = 0, err, len, n;
+	size_t to_read = iov_iter_count(&subreq->io_iter);
 
 	if (S_ISLNK(rreq->inode->i_mode)) {
 		/* p9_client_readlink() must not be called for legacy protocols
@@ -105,6 +106,8 @@ static void v9fs_issue_read(struct netfs_io_subrequest *subreq)
 		__set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
 	if (pos + total >= i_size_read(rreq->inode))
 		__set_bit(NETFS_SREQ_HIT_EOF, &subreq->flags);
+	if (!err && !total && to_read)
+		__set_bit(NETFS_SREQ_HIT_EOF, &subreq->flags);
 	if (!err && total) {
 		subreq->transferred += total;
 		__set_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH] 9p: treat read return values of 0 as EOF

[Dominique Martinet]

Barret Rhoden wrote on Thu, Jul 02, 2026 at 09:09:32AM +0000:
> If your 9p server shares a synthetic file system such as sysfs, the file
> size from stat often disagrees with the actual file size.
> 
> Treat non-error reads of 0 as EOF, except in the case where the request
> was for 0 bytes.

> Tested:
> 	git@github.com:hugelgupf/p9.git, served "/" on 10.0.2.2
> 
> 	bash-5.3# mount -t 9p -o trans=tcp,port=12345,cache=none 10.0.2.2 /tmp/foo
> 	bash-5.3# cat /tmp/foo/sys/devices/system/cpu/online
> 	0-47

David,
that looks like something that should be handled in netfs - is it right
to do this on the 9p side?

I've just tried with qemu and with cache=none I get ENODATA, with
cache=loose I get the data followed by 4094 zeroes...

Interestingly there also are files with 0 size e.g.
/sys/devices/virtual/net/br0/brforward,
and for this one with cache=none I can read it fine, but with
cache=loose I get an empty file.


Barret,
thanks for the patch, please give us a bit of time to check; I'm
sometimes slow to reply (like for your sending the patch first without
the [PATCH] tag in subject) but I did see it, just hard to find time to
review...

Cheers,
-- 
Dominique

Re: [PATCH] 9p: treat read return values of 0 as EOF

[Barret Rhoden]

On 7/3/26 10:28 AM, Dominique Martinet wrote:
...
> Barret,
> thanks for the patch, please give us a bit of time to check; I'm
> sometimes slow to reply (like for your sending the patch first without
> the [PATCH] tag in subject) but I did see it, just hard to find time to
> review...

no problem - i thought it might have gotten missed due to the lack of 
[PATCH].  my bad on that.

let me know if i can test anything too.

thanks,
barret

Re: [PATCH] 9p: treat read return values of 0 as EOF

[David Howells]

This is kind of a weird situation.  We're caching locally the content of files
aren't really regular files and probably shouldn't be cached.  I'm not sure
what the best way to deal with that is.  I wonder if there's some way to
detect that and mark then non-cacheable.  (Assuming the server can be told not
to even serve them).

Can we detect that the EOF length doesn't match i_size and set a flag to say
"don't cache" in netfs_inode::flags?

David

Re: [PATCH] 9p: treat read return values of 0 as EOF

[Barret Rhoden]

On 7/13/26 5:29 AM, David Howells wrote:
> This is kind of a weird situation.  We're caching locally the content of files
> aren't really regular files and probably shouldn't be cached.  I'm not sure
> what the best way to deal with that is.  I wonder if there's some way to
> detect that and mark then non-cacheable.  (Assuming the server can be told not
> to even serve them).
> 
> Can we detect that the EOF length doesn't match i_size and set a flag to say
> "don't cache" in netfs_inode::flags?
Possibly, but could you have false positives from this?  e.g. if a 
file's size is changed concurrently with a read returning EOF?

As far as detecting EOF in the first place, my patch had the 9p client 
doing it.  Not sure, but Dominique's question might have been whether 
netfs should have done the detecting instead?

 From what I can see, the netfs clients were responsible for setting EOF 
(except in netfs_clear_unread()).  Some in response to an ENODATA error, 
others due to the "did we read past the end of the file size."  Not sure 
whose responsibility it is to detect these cases: netfs or the FSes 
themselves.

Thanks,
Barret

Re: [PATCH] 9p: treat read return values of 0 as EOF

[Dominique Martinet]

Sorry for dropping the ball here, I haven't had any time to do 9p stuff
at all recently and the wave of LLM generated patches hasn't helped...

Barret Rhoden wrote on Mon, Jul 27, 2026 at 01:38:16PM -0400:
> On 7/13/26 5:29 AM, David Howells wrote:
> > This is kind of a weird situation.  We're caching locally the content of files
> > aren't really regular files and probably shouldn't be cached.  I'm not sure
> > what the best way to deal with that is.  I wonder if there's some way to
> > detect that and mark then non-cacheable.  (Assuming the server can be told not
> > to even serve them).
> > 
> > Can we detect that the EOF length doesn't match i_size and set a flag to say
> > "don't cache" in netfs_inode::flags?
> Possibly, but could you have false positives from this?  e.g. if a file's
> size is changed concurrently with a read returning EOF?
> 
> As far as detecting EOF in the first place, my patch had the 9p client doing
> it.  Not sure, but Dominique's question might have been whether netfs should
> have done the detecting instead?
>
> From what I can see, the netfs clients were responsible for setting EOF
> (except in netfs_clear_unread()).  Some in response to an ENODATA error,
> others due to the "did we read past the end of the file size."  Not sure
> whose responsibility it is to detect these cases: netfs or the FSes
> themselves.

Right, that was my question; I honestly just don't understand the
requirement/promises of the netfs layer.

In this particular case we have two patterns to address:
- synthetic files with stat size > actual size,
e.g. /sys/devices/system/cpu/online
- synthetic files with stat size = 0 (rarer),
e.g. /sys/devices/virtual/net/br0/brforward

For two at least two modes:
- cache=none
- cache=loose (writeback I guess)

Right now, none of these work, with various degrees of not working;
I'm not sure we can do much about some of them (e.g. stat size=0 with
cache enabled I'm fine getting empty data), but imo
- cache=none should just try reading as userspace request anyway and
stop as soon as the server returns 0, that's not a 9p specific behavior
and I wouldn't have expected having to do any special handling for it.
(but if you tell me it's to be done in 9p case, then sure, we'll do it
here)
- cache=writeback + stat size > read size should also stop when the
server returns 0 and truncate its view of the size to that; the problem
in 9p is that we'll reset the size again to the remote size shortly
afterwards so that will need more thinking (and overlaps with the other
cache size bugs we have in flight), so let's focus on cache=none for now

Thanks,
-- 
Dominique Martinet | Asmadeus