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

[Barret Rhoden <brho@google.com>]

On 9/13/26 11:49 AM, Dominique Martinet wrote:
> 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...

no problem.

as far as resolving the underlying netfs stuff goes, i don't know how 
much of the logic about "are we at EOF" should be in netfs or in the 
servers (9p).

would you be OK with merging my original patch so that at least 9p works?

thanks,
barret



> 
> 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,