I did not set .owner... I have looked to see how others have set it... I will set it, retest and report back here for completeness... -Mike On Sun, Nov 13, 2016 at 1:51 PM, Nicolai Stange <nicstange@xxxxxxxxx> wrote: > Hi again, > > bad news: my previous analysis was completely wrong, c.f. below. > Good news (from my point of view): debugfs is correct, no fix needed for > it. > > Apologies for the confusion... > > > Nicolai Stange <nicstange@xxxxxxxxx> writes: > >> Greg KH <greg@xxxxxxxxx> writes: >> >>> On Mon, Oct 31, 2016 at 02:32:56PM -0400, Mike Marshall wrote: >>> >>>> But... really bad things happen if someone unloads the Orangefs >>>> module after my test program does the open and before the read >>>> starts. So I picked another debugfs-using-filesystem (f2fs) and >>>> pointed my tester-program at /sys/kernel/debug/f2fs/status, and >>>> the same bad thing happens there. >> >> [...] >> >>>> [ 1240.144316] Call Trace: >>>> [ 1240.144450] [<ffffffff8122907f>] __fput+0xdf/0x1d0 >>>> [ 1240.144704] [<ffffffff812291ae>] ____fput+0xe/0x10 >>>> [ 1240.144962] [<ffffffff810b97de>] task_work_run+0x8e/0xc0 >>>> [ 1240.145243] [<ffffffff8109b98e>] do_exit+0x2ae/0xae0 >> >> >> Thank you very much for this detailed report! >> >> At least for the .../f2fs/status file, your splat at fput() can be >> readily explained with the full proxy's releaser not being protected >> against file removals in any way. >> >> Partly this is on purpose, c.f. the comment in full_proxy_release(). >> >> However, I should have at least tried to acquire a reference to the >> owning module before accessing some static struct file_operations or >> even calling some ->release() within it. Meh. > > This is what I got wrong: debugfs does acquire the needed references > correctly (details below). For the case of f2fs' "status" file, the > file_operations ->owner is simply not set as it should have been, > i.e. to THIS_MODULE. > > > Details on debugfs' reference acquisition: > The open proxy, full_proxy_open(), gets a reference to the "real" > file_operations, hence to its module. (Only in its error path it > releases it again). > > full_proxy_release() is in charge of dropping that reference again, but > only *after* it has attempted to call the "real" ->release(). > > So, as long as a file has been (successfully) opened, a reference to the > original file_operation's ->owner is owned, preventing it from getting > unloaded. > > > Can you confirm that you didn't set ->owner in your Orangefs related > tests, too? > > > Thanks, > > Nicolai -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
