On 17.05.2018 14:31, Pavel Butsykin wrote: > On 17.05.2018 13:05, Kirill Tkhai wrote: >> Hi, Pasha, >> >> On 17.05.2018 12:40, Pavel Butsykin wrote: >>> Allow initialization of kdirect only for vstorage/pstorage. >>> >>> Signed-off-by: Pavel Butsykin <pbutsy...@virtuozzo.com> >>> --- >>> drivers/block/ploop/dev.c | 4 ---- >>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 +++++++++++++++++------ >>> include/linux/fs.h | 5 +++++ >>> 3 files changed, 22 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c >>> index 0e72656ec8f9..c6094144c7a5 100644 >>> --- a/drivers/block/ploop/dev.c >>> +++ b/drivers/block/ploop/dev.c >>> @@ -3880,10 +3880,6 @@ static int ploop_truncate(struct ploop_device * plo, >>> unsigned long arg) >>> return err; >>> } >>> -#define IS_PSTORAGE(sb) (sb->s_magic == FUSE_SUPER_MAGIC && \ >>> - (!strcmp(sb->s_subtype, "pstorage") || \ >>> - !strcmp(sb->s_subtype, "vstorage"))) >>> - >>> static int ploop_bd_full(struct backing_dev_info *bdi, long long nr, int >>> root) >>> { >>> struct ploop_device *plo = bdi->congested_data; >>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c >>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c >>> index b8503d55e246..4b9c4a304571 100644 >>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c >>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c >>> @@ -144,13 +144,24 @@ void kpcs_conn_abort(struct fuse_conn *fc) >>> } >>> static int kpcs_probe(struct fuse_conn *fc, char *name) >>> - >>> { >>> - printk("%s TODO IMPLEMENT check fuse_conn args here!\n", __FUNCTION__); >>> - if (!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) >>> - return 1; >>> + if (strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) { >>> + printk(KERN_ERR "FUSE: kio_pcs: invalid kio_ops name: %s\n", name); >> >> Despite there was printk() without error level modifier (which is not >> correct), >> and printk(KERN_ERR) is better, let's follow the modern Linux kernel style, >> which anyway will be requested in case of submitting patches to mainstream. >> pr_err() is more likely to use for such the printing (see the rest of pr_* >> in printk.h). >> > > ok, thanks. > >>> + return 0; >>> + } >>> - return 0; >>> + if (!(fc->flags & FUSE_KDIRECT_IO)) { >>> + printk(KERN_ERR "FUSE: kio_pcs: kdirect is not enabled\n"); >>> + return 0; >>> + } >> >> I don't think this is needed, since we already check for this bit in >> fuse_fill_super() >> before call of fuse_kio_get(). But I'm not insist on this. >> > > And what is the meaning of this check: > "!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)" ?
FUSE_KIO_NAME looks like a define of string, but it's a size, and it was confusing for me. Possible, we can drop this too. Less code is better for the attention. > We also already have this check. I assumed this can protect against > possible mistakes in the future. I think, we fastly find the root cause in case of something is wrong on mount. Duplications confuse... But I'm not insist on this. >>> + >>> + if (!IS_PSTORAGE(fc->sb)) { >>> + printk(KERN_ERR "FUSE: kio_pcs: kdirect is only available for" >>> + "pstorage/vstorage fuse mount\n"); >>> + return 0; >>> + } >>> + >>> + return 1; >>> } >>> @@ -1202,7 +1213,7 @@ err: >>> static struct fuse_kio_ops kio_pcs_ops = { >>> .name = "pcs", >>> .owner = THIS_MODULE, >>> - .probe = kpcs_probe, /*TODO: check sb->dev name */ >>> + .probe = kpcs_probe, >>> .conn_init = kpcs_conn_init, >>> .conn_fini = kpcs_conn_fini, >>> diff --git a/include/linux/fs.h b/include/linux/fs.h >>> index a1dc3521f979..21770550b6c1 100644 >>> --- a/include/linux/fs.h >>> +++ b/include/linux/fs.h >>> @@ -1713,6 +1713,11 @@ struct super_block { >>> struct list_lru s_inode_lru ____cacheline_aligned_in_smp; >>> }; >>> +#define IS_PSTORAGE(sb) ((sb)->s_magic == FUSE_SUPER_MAGIC && \ >>> + (sb)->s_subtype && \ >>> + (!strcmp((sb)->s_subtype, "pstorage") || \ >>> + !strcmp((sb)->s_subtype, "vstorage"))) >>> + >>> extern const unsigned super_block_wrapper_version; >>> struct super_block_wrapper { >>> struct super_block sb; >>> >> >> Thanks, >> Kirill >> _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel