Re: [HACK] convert i_alloc_sem for direct_io.c craziness!

[Peter Zijlstra]

On Mon, 24 Sep 2007 17:14:26 -0400 (EDT) Steven Rostedt
<rostedt@xxxxxxxxxxx> wrote:

> 
> Hopefully I will get some attention from those that are responsible for
> fs/direct_io.c
> 
> Ingo and Thomas,
> 
> This patch converts the i_alloc_sem into a compat_rw_semaphore for the -rt
> patch.  Seems that the code in fs/direct_io.c does some nasty logic with
> the i_alloc_sem.  For DIO_LOCKING, I'm assuming that the i_alloc_sem is
> used as a reference counter for pending requests. When the request is
> made, the down_read is performed. When the request is handled by the block
> softirq, then that softirq does an up on the request. So the owner is not
> the same between down and up. When all requests are handled, the semaphore
> counter should be zero. This keeps away any write access while requests
> are pending.
> 
> Now this may all be well and dandy for vanilla Linux, but it breaks
> miserbly when converted to -rt.
> 
> 1) In RT rw_semaphores must be up'd by the same thread that down's it.
> 
> 2) We can't do PI on the correct processes.
> 
> This patch converts (for now) the i_alloc_sem into a compat_rw_semaphore
> to give back the old features to the sem. This fixes deadlocks that we've
> been having WRT direct_io.  But unfortunately, it now opens up
> unbonded priority inversion with this semaphore.  But really, those that
> can be affected by this, shouldn't be doing disk IO anyway.
> 
> The real fix would be to get rid of the read semaphore trickery in
> direct_io.c.

How about teaching {up,down}_read_non_owner() to barf on rw_semaphore
in -rt?

> Signed-off-by: Steve Rostedt <rostedt@xxxxxxxxxxx>
> 
> Index: linux-2.6.23-rc4-rt1/include/linux/fs.h
> ===================================================================
> --- linux-2.6.23-rc4-rt1.orig/include/linux/fs.h	2007-09-24 16:58:59.000000000 -0400
> +++ linux-2.6.23-rc4-rt1/include/linux/fs.h	2007-09-24 16:59:11.000000000 -0400
> @@ -577,7 +577,7 @@ struct inode {
>  	umode_t			i_mode;
>  	spinlock_t		i_lock;	/* i_blocks, i_bytes, maybe i_size */
>  	struct mutex		i_mutex;
> -	struct rw_semaphore	i_alloc_sem;
> +	struct compat_rw_semaphore	i_alloc_sem;
>  	const struct inode_operations	*i_op;
>  	const struct file_operations	*i_fop;	/* former ->i_op->default_file_ops */
>  	struct super_block	*i_sb;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/