> On Aug 21, 2024, at 10:00 PM, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > Hey Jeff, > > On Wed, Aug 21, 2024 at 03:20:55PM -0400, Jeff Layton wrote: >> >> This looks much improved. I didn't see anything that stood out at me as >> being problematic code-wise with the design or final product, aside >> from a couple of minor things. > > BTW, thanks for this feedback, much appreciated! > >> But...this patchset is hard to review. My main gripe is that there is a >> lot of "churn" -- places where you add code, just to rework it in a new >> way in a later patch. >> >> For instance, the nfsd_file conversion should be integrated into the >> new infrastructure much earlier instead of having a patch that later >> does that conversion. Those kinds extraneous changes make this much >> harder to review than it would be if this were done in a way that >> avoided that churn. > > I think I've addressed all your v12 review comments from earlier > today. I've pushed the new series out to my git repo here: > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next > > No code changes, purely a bunch of rebasing to clean up like you > suggested. Only outstanding thing is the nfsd tracepoints handling of > NULL rqstp (would like to get Chuck's expert feedback on that point). > > Please feel free to have a look at my branch while I wait for any > other v12 feedback from Chuck and/or others before I send out v13. > I'd like to avoid spamming the list like I did in the past ;) Was looking for an appropriate place to reply with this question, but didn't see one. So here goes: One of the issues Neil mentioned was dealing with the case where a server file system is unexported and perhaps unmounted while there is LOCALIO ongoing. Can you describe what the client and application see in this case? Do you have test cases for this scenario? Obviously we don't want a crash or deadlock, but I would guess the client/app should behave just like remote NFSv3 -- there, the server returns ESTALE on a READ or WRITE, and read(2) or write(2) on the client returns EIO. Ie, behavior should be deterministic. -- Chuck Lever
