Re: [PATCH] nvmet: introduce use_vfs ns-attr

[Mark Ruijter <MRuijter@onestopsystems.com>]

Hi Keith,

You are right, that check and the goto should be included.

Note that I wrote this patch to prove that a performance problem exists when using raid1.
Either the md raid1 driver or the io-cmd-bdev.c code has issues.
When you add an additional layer like the VFS the performance should typically drop with 5~10%.
However in this case the performance increases even though the nvme target uses direct-io and the random writes do not get merged by the VFS.

Thanks,

Mark Ruijter
 
Op 24-10-19 04:00 heeft Keith Busch <kbusch@kernel.org> geschreven:

    On Wed, Oct 23, 2019 at 01:17:15PM -0700, Chaitanya Kulkarni wrote:
    > @@ -45,17 +46,27 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
    >  
    >  	ret = vfs_getattr(&ns->file->f_path,
    >  			&stat, STATX_SIZE, AT_STATX_FORCE_SYNC);
    > -	if (ret)
    > -		goto err;
    > -
    > -	ns->size = stat.size;
    > -	/*
    > -	 * i_blkbits can be greater than the universally accepted upper bound,
    > -	 * so make sure we export a sane namespace lba_shift.
    > -	 */
    > -	ns->blksize_shift = min_t(u8,
    > -			file_inode(ns->file)->i_blkbits, 12);
    > +	if (ret) {
    > +		pr_err("failed to stat device file %s\n",
    > +			ns->device_path);
    
    
    Why remove the 'goto err' from this condition? The code that proceeds
    may be using an uninitialized 'stat' without it.
    
    
    > +	}
    >  
    > +	if (stat.size == 0 && ns->use_vfs) {
    > +		bdev = blkdev_get_by_path(ns->device_path,
    > +					  FMODE_READ | FMODE_WRITE, NULL);
    > +		if (IS_ERR(bdev))
    > +			goto err;
    > +		ns->size = i_size_read(bdev->bd_inode);
    > +		ns->blksize_shift = blksize_bits(bdev_logical_block_size(bdev));
    > +	} else {
    > +		/*
    > +		 * i_blkbits can be greater than the universally accepted upper
    > +		 * bound, so make sure we export a sane namespace lba_shift.
    > +		 */
    > +		ns->size = stat.size;
    > +		ns->blksize_shift = min_t(u8,
    > +				file_inode(ns->file)->i_blkbits, 12);
    > +	}
    

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme