On Wed, Jul 13, 2022 at 3:17 AM Steve French <smfrench@xxxxxxxxx> wrote: > > Looks like the patch Shyam had for force unmount doesn't help regular unmount case but fixing the reference count on super lock could fix it. When file is closed locally but not over smb (deferred close) we can probably decrement the reference count so unmount would work > > On Tue, Jul 12, 2022, 16:40 ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: >> >> On Sat, 9 Jul 2022 at 16:48, Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: >> > >> > On Sat, Jul 9, 2022 at 11:45 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: >> > > >> > > Hi Steve/Ronnie, >> > > >> > > I'm seeing a strange problem with deferred close. >> > > This is the test that I'm running: >> > > 1. mount a share with actime=600 >> > > 2. open a file in the share so that a handle is opened, and it gets >> > > scheduled for deferred close. >> > > 3. umount the share. That works. >> > > 4. rmmod cifs does not seem to work. It says module cifs is in use. >> > > DebugData shows that tcon/ses/connections are all active for the >> > > unmounted share. >> > > >> > > I think I understand why this happens, but need help understanding how >> > > to fix this. >> > > >> > > Each handle open takes a reference on tcon, and also on the >> > > superblock. So even when the mount point is umounted, tcon does not >> > > get freed. I see that it gets freed when all handles that are deferred >> > > for close are actually closed. >> > > >> > > I tried calling cifs_close_all_deferred_files for the tcon in >> > > cifs_umount_begin. I even tried printing a log there. I did not see >> > > that getting logged during umount. Does that mean umount_begin is not >> > > getting called? >> > > >> > > Wondering what is the correct way to fix this? Ideally, we should call >> > > cifs_close_all_deferred_files as soon as the share is umounted. Which >> > > is the first callback in cifs.ko for this. I was assuming that >> > > umount_begin is that callback. But my experiments are not seeing that >> > > getting called. >> > > >> > > -- >> > > Regards, >> > > Shyam >> > >> > I checked the VFS code. It looks like the umount_begin callback gets >> > called only when called with "umount --force" option. >> > I tested that. It seems to work. >> > >> > Is it the expected though without force? Doesn't seem right to me. >> >> >> umount_begin() is a special callback just for network filesystems >> that need to do something special >> if/when a forced umount is attempted so this would be the wrong place >> for the normal deferred close >> handling. It is poorly documented though :-( >> >> Maybe cifs_kill_sb() would be the right place to use? >> >> >> > >> > -- >> > Regards, >> > Shyam Thanks Ronnie and Steve. This seems to work: https://github.com/sprasad-microsoft/smb3-kernel-client/commit/c1aa4ca2231a247abcf948dd7411ea5a1a0330b8.patch Please review the changes. Tested some basic test cases. Seems to work. I don't see this change affecting any other mount options. Do you? -- Regards, Shyam
