Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

xfs: fix race between healthmon unmount and read_iter

xfs/1879 on one of my test VMs got stuck due to the xfs_io healthmon
subcommand sleeping in wait_event_interruptible at:

xfs_healthmon_read_iter+0x558/0x5f8 [xfs]
vfs_read+0x248/0x320
ksys_read+0x78/0x120

Looking at xfs_healthmon_read_iter, in !O_NONBLOCK mode it will sleep
until the mount cookie == DETACHED_MOUNT_COOKIE, there are events
waiting to be formatted, or there are formatted events in the read
buffer that could be copied to userspace.

Poking into the running kernel, I see that there are zero events in the
list, the read buffer is empty, and the mount cookie is indeed in
DETACHED state. IOWs, xfs_healthmon_has_eventdata should have returned
true, but instead we're asleep waiting for a wakeup.

I think what happened here is that xfs_healthmon_read_iter and
xfs_healthmon_unmount were racing with each other, and _read_iter lost
the race. _unmount queued an unmount event, which woke up _read_iter.
It found, formatted, and copied the event out to userspace. That
cleared out the pending event list and emptied the read buffer. xfs_io
then called read() again, so _has_eventdata decided that we should sleep
on the empty event queue.

Next, _unmount called xfs_healthmon_detach, which set the mount cookie
to DETACHED. Unfortunately, it didn't call wake_up_all on the hm, so
the wait_event_interruptible in the _read_iter thread remains asleep.
That's why the test stalled.

Fix this by moving the wake_up_all call to xfs_healthmon_detach.

Fixes: b3a289a2a9397b ("xfs: create event queuing, formatting, and discovery infrastructure")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

authored by

Darrick J. Wong and committed by
Carlos Maiolino
0ca1a833 6270b8ac

+10 -7
+10 -7
fs/xfs/xfs_healthmon.c
··· 141 141 hm->mount_cookie = DETACHED_MOUNT_COOKIE; 142 142 spin_unlock(&xfs_healthmon_lock); 143 143 144 + /* 145 + * Wake up any readers that might remain. This can happen if unmount 146 + * races with the healthmon fd owner entering ->read_iter, having 147 + * already emptied the event queue. 148 + * 149 + * In the ->release case there shouldn't be any readers because the 150 + * only users of the waiter are read and poll. 151 + */ 152 + wake_up_all(&hm->wait); 153 + 144 154 trace_xfs_healthmon_detach(hm); 145 155 xfs_healthmon_put(hm); 146 156 } ··· 1037 1027 * process can create another health monitor file. 1038 1028 */ 1039 1029 xfs_healthmon_detach(hm); 1040 - 1041 - /* 1042 - * Wake up any readers that might be left. There shouldn't be any 1043 - * because the only users of the waiter are read and poll. 1044 - */ 1045 - wake_up_all(&hm->wait); 1046 - 1047 1030 xfs_healthmon_put(hm); 1048 1031 return 0; 1049 1032 }