Re: Locking problems with Linux 4.9 and 4.11 with NFSD and `fs/iomap.c`

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Christoph, dear Markus,


On 08/11/17 12:15, Christoph Hellwig wrote:
> On Thu, Aug 10, 2017 at 07:54:51PM +0000, Markus Stockhausen wrote:
>> Lets say you are trying to zero multiple of 4GB chunks. With bytes
>> evaluated towards 0 this will hit an endless loop within that iomap
>> function. That might explain your observation. If that is right a bugfix
>> would qualify for stable 4.8+
> 
> Yes, it seems like min_t casts arguments 2 and 3 to the type in argument
> 1, which could lead to incorrect truncation.
> 
> Paul, please try the patch below:
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 039266128b7f..59cc98ad7577 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -278,7 +278,7 @@ iomap_dirty_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>   		unsigned long bytes;	/* Bytes to write to page */
>   
>   		offset = (pos & (PAGE_SIZE - 1));
> -		bytes = min_t(unsigned long, PAGE_SIZE - offset, length);
> +		bytes = min_t(loff_t, PAGE_SIZE - offset, length);
>   
>   		rpage = __iomap_read_page(inode, pos);
>   		if (IS_ERR(rpage))
> @@ -373,7 +373,7 @@ iomap_zero_range_actor(struct inode *inode, loff_t pos, loff_t count,
>   		unsigned offset, bytes;
>   
>   		offset = pos & (PAGE_SIZE - 1); /* Within page */
> -		bytes = min_t(unsigned, PAGE_SIZE - offset, count);
> +		bytes = min_t(loff_t, PAGE_SIZE - offset, count);
>   
>   		if (IS_DAX(inode))
>   			status = iomap_dax_zero(pos, offset, bytes, iomap);

I applied this on top of Linux 4.9.41. Even when writing 40 100 GB in 
parallel on an NFS exported directory from different systems, and 
keeping the load high on the system, the NFS exported directory was 
always accessible from all hosts.

Markus, thank you very much for looking into this and spotting the bug. 
A bug thank you to all people helping to analyze this issue.

I’ll keep you posted, but until know.

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul