Re: [PATCH] vfs: introduce UMOUNT_WAIT which waits for umount completion

From: Jaegeuk Kim
Date: Fri Sep 15 2017 - 14:44:40 EST

On 09/15, Al Viro wrote:
> On Thu, Sep 14, 2017 at 08:45:18PM -0700, Jaegeuk Kim wrote:
> > > Which filesystem it is? With root I would've expected remount ro done
> > > by sys_umount(); with anything else... How has it managed to avoid
> > > -EBUSY? If it was umount -l (IOW, MNT_DETACH), I can see that happening,
> > > but... How would flushing prevent the scenario when the same opened
> > > file had remained open until after the umount(2) return?
> >
> > It's ext4, and we use umount(0) and retry it several times if -EBUSY happens.
> ????
> umount(0) will result in EFAULT; what are you talking about?

Sorry, I meant: umount2("/data", 0);

> > But, I don't see -EBUSY error in the log.
> Sorry, I'd been unclear - where is it mounted? Is that the root filesystem?

No, it's /userdata in Android.

> > > In other words, where has that fput() come from and how had it managed
> > > to get past the umount(2)?
> >
> > Huge number of fput() were called by system drivers when init kills all the
> > processes before umount(2). So, most of fput() were added in delayed_fput_list.
> Umm... What do you mean by system drivers? If it was held by userland processes,
> then we are back to the same question - why has task_work_add() failed in fput()?
> If it had been kernel threads, which files had they been holding open?

So, I digged it in more detail, and found, in drivers/android/binder.c [1],
- binder_ioctl()
- create a kernel thread
- zombie_cleanup_check()
- binder_defer_work()
- queue_work(..., &binder_deferred_work);

- binder_deferred_func()
- binder_clear_zombies()
- binder_proc_clear_zombies()
- put_files_struct()
- close_files()
- filp_close()
- fput()

It seems binder holds some proc files. If you think it's android-specific issue,
I may need to write a patch for android kernel instead. Let me know.


> > Then, it seems there is a race between delayed_fput() and umount(). Anyway,
> > even after umount returns zero, it seems ext4's superblock is still alive
> > and waiting for delayed_fput() which will finally call put_super.
> That might be more than one mount of the same fs (in different namespaces, for
> example) with umount taking out one of those, with the other having been
> hit with umount -l before that, with some opened files being the only thing
> that used to keep it alive.
> I'd like to see /proc/1/mountinfo and fuser output, TBH... I'm not familiar enough
> with Android userland setup, so my apologies for dumb questions ;-/