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
