On Sun, 29 Jun 2014 17:36:50 +0400, Vyacheslav Dubeyko wrote:
> From: Vyacheslav Dubeyko <Vyacheslav.Dubeyko@xxxxxxxx>
> Subject: [PATCH] nilfs2: fix issue with potential infinite loop in nilfs_mdt_get_block()
>
> Potentially, there are situations when nilfs_mdt_read_block() can
> return -ENOENT and nilfs_mdt_create_block() can return -EEXIST.
Did you actually see this situation happened ?
nilfs_mdt_read_block() returns -ENOENT only if nilfs_grab_buffer()
didn't find a buffer head on page cache and nilfs_bmap_lookup() hit
a hole block.
nilfs_mdt_create_block() returns -EEXIST only if nilfs_grab_buffer()
got a buffer head in uptodate state or nilfs_bmap_insert() failed due
to an existing block.
So, these usually don't happen at the same time. What situation
do you suppose ?
This function is so primitive to easily change even though the retry
loop looks an interim implementation of an early date. I would
understand what is going on and whole impact of the change.
> Such situation results in infinite loop inside nilfs_mdt_get_block()
> method. This patch fixes issue with potential infinite loop in
> nilfs_mdt_get_block() by means of uncomment limitation of read-create
> loop retries.
If this patch is applied, nilfs_mdt_get_block() can return -ENOENT
even for the case of create == true. This is unexpected for caller
functions. I think we should return proper error code which fits to
the exceptional situation that causes an infinite repetition (if it
actually happens).
Regards,
Ryusuke Konishi
> Signed-off-by: Vyacheslav Dubeyko <Vyacheslav.Dubeyko@xxxxxxxx>
> CC: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> CC: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxxxxxx>
> ---
> fs/nilfs2/mdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
> index c4dcd1d..d1e121f 100644
> --- a/fs/nilfs2/mdt.c
> +++ b/fs/nilfs2/mdt.c
> @@ -254,7 +254,7 @@ int nilfs_mdt_get_block(struct inode *inode, unsigned long blkoff, int create,
>
> ret = nilfs_mdt_create_block(inode, blkoff, out_bh, init_block);
> if (unlikely(ret == -EEXIST)) {
> - /* create = 0; */ /* limit read-create loop retries */
> + create = 0; /* limit read-create loop retries */
> goto retry;
> }
> return ret;
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
