Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block

2021-07-15 Thread Jason Wang 


在 2021/7/15 下午5:47, Yury Kamenev 写道:

Signed-off-by: Yury Kamenev 



I think we need a better commit log here.

And why do we need a Kconfig for this? If there's a good reason, I guess 
the right approach is to invent something in the virtio core (via /sys)?


Thanks



---
  .../admin-guide/kernel-parameters.txt |  3 +++
  drivers/block/Kconfig |  7 +
  drivers/block/virtio_blk.c| 26 +++
  include/uapi/linux/virtio_blk.h   |  2 ++
  4 files changed, 38 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..941bdaf5c167 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6076,6 +6076,9 @@
brightness level.
default: 1

+   virtiopartscan
+   Enable virtio block device partition scanning omission based on 
VIRTIO_BLK_F_NO_PART_SCAN feature flag.
+
virtio_mmio.device=
[VMMIO] Memory mapped virtio (platform) device.

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 63056cfd4b62..69ecd3fd7037 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -399,6 +399,13 @@ config VIRTIO_BLK
  This is the virtual block driver for virtio.  It can be used with
QEMU based VMMs (like KVM or Xen).  Say Y or M.

+config VIRTIO_BLK_NO_PART_SCAN
+   bool "Disable partition scanning for devices with no partitions"
+   depends on VIRTIO_BLK
+   help
+ Disable partition scanning for devices with no partitions.
+ Can reduce the kernel start time for tiny systems like squashfs 
images.
+
  config BLK_DEV_RBD
tristate "Rados block device (RBD)"
depends on INET && BLOCK
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 4b49df2dfd23..479711d3791c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -692,6 +692,19 @@ static const struct blk_mq_ops virtio_mq_ops = {
  static unsigned int virtblk_queue_depth;
  module_param_named(queue_depth, virtblk_queue_depth, uint, 0444);

+#ifndef MODULE
+#ifdef CONFIG_VIRTIO_BLK_NO_PART_SCAN
+static int partitions_scanning_disable __read_mostly;
+
+static int __init partitions_scanning_setup(char *__unused)
+{
+   partitions_scanning_disable = 1;
+   return 1;
+}
+__setup("nopartscan", partitions_scanning_setup);
+#endif
+#endif
+
  static int virtblk_probe(struct virtio_device *vdev)
  {
struct virtio_blk *vblk;
@@ -790,6 +803,13 @@ static int virtblk_probe(struct virtio_device *vdev)
vblk->disk->flags |= GENHD_FL_EXT_DEVT;
vblk->index = index;

+#ifdef CONFIG_VIRTIO_BLK_NO_PART_SCAN
+   if (unlikely(partitions_scanning_disable))
+   /* disable partitions scanning if it was stated in virtio 
features*/
+   if (virtio_has_feature(vdev, VIRTIO_BLK_F_NO_PART_SCAN))
+   vblk->disk->flags |= GENHD_FL_NO_PART_SCAN;
+#endif
+
/* configure queue flush support */
virtblk_update_cache_mode(vdev);

@@ -966,6 +986,9 @@ static unsigned int features_legacy[] = {
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
+#ifdef CONFIG_VIRTIO_BLK_NO_PART_SCAN
+   VIRTIO_BLK_F_NO_PART_SCAN,
+#endif
  }
  ;
  static unsigned int features[] = {
@@ -973,6 +996,9 @@ static unsigned int features[] = {
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
+#ifdef CONFIG_VIRTIO_BLK_NO_PART_SCAN
+   VIRTIO_BLK_F_NO_PART_SCAN,
+#endif
  };

  static struct virtio_driver virtio_blk = {
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index d888f013d9ff..9b381675342a 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -40,6 +40,7 @@
  #define VIRTIO_BLK_F_MQ   12  /* support more than one vq */
  #define VIRTIO_BLK_F_DISCARD  13  /* DISCARD is supported */
  #define VIRTIO_BLK_F_WRITE_ZEROES 14  /* WRITE ZEROES is supported */
+#define VIRTIO_BLK_F_NO_PART_SCAN  16  /* Disable partition scanning */

  /* Legacy feature bits */
  #ifndef VIRTIO_BLK_NO_LEGACY
--
2.24.3 (Apple Git-128)



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH 3/3] fuse: add per-file DAX flag

2021-07-15 Thread JeffleXu 



On 7/16/21 9:32 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
>>
>>
>> On 7/16/21 8:51 AM, Vivek Goyal wrote:
>>> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
 On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> this file.
>
> When the per-file DAX flag changes for an *opened* file, the state of
> the file won't be updated until this file is closed and reopened later.
>
> Currently it is not implemented yet to change per-file DAX flag inside
> guest kernel, e.g., by chattr(1).

 Thanks for the patch, it looks good to me.

 I think it's a good starting point, what I'd like to discuss here is
 whether we're going to let chattr to toggle the dax flag.
>>>
>>> I have the same question. Why not take chattr approach as taken
>>> by ext4/xfs as well.
>>>
>>> Vivek
>>
>> Thanks.
>>
>> We can implement the chattr approach as ext4/xfs do, if we have this use
>> scenario. It's an RFC patch, and I want to collect more feedback as soon
>> as possible.
> 
> I guess chattr approach will allow client (as well as server) to control
> which files should be DAX. While this approach allows only server to
> specify which files should use DAX. Given currently we let client
> control whether to use dax or not (-o dax), it probably will make
> sense to use chattr based approach?

Yes, changing the per-file DAX flag from guest side, such as by chattr,
may be needed for completeness. I will include this in the next version.

> 
> I will look at the patches. Do you have a corresponding user space
> implementation somewhere so that I can test it?

Thanks. I have sent the corresponding patch in-reply-to your mail.

-- 
Thanks,
Jeffle
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] virtiofsd: support per-file DAX

2021-07-15 Thread Jeffle Xu 
An example implementation of supporting per-file DAX flag for virtiofsd,
where DAx is enabled for files larger than 1M size.

Signed-off-by: Jeffle Xu 
---
 contrib/virtiofsd/fuse_kernel.h   | 4 +++-
 contrib/virtiofsd/fuse_lowlevel.c | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
index d2b7ccf96b..9c476b7021 100644
--- a/contrib/virtiofsd/fuse_kernel.h
+++ b/contrib/virtiofsd/fuse_kernel.h
@@ -165,6 +165,8 @@
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
 
+#define FUSE_ATTR_DAX  (1 << 1)
+
 /* Make sure all structures are padded to 64bit boundary, so 32bit
userspace works under 64bit kernels */
 
@@ -184,7 +186,7 @@ struct fuse_attr {
uint32_tgid;
uint32_trdev;
uint32_tblksize;
-   uint32_tpadding;
+   uint32_tflags;
 };
 
 struct fuse_kstatfs {
diff --git a/contrib/virtiofsd/fuse_lowlevel.c 
b/contrib/virtiofsd/fuse_lowlevel.c
index 046a1b4a02..d8a3873246 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -60,6 +60,9 @@ static void convert_stat(const struct stat *stbuf, struct 
fuse_attr *attr)
attr->atimensec = ST_ATIM_NSEC(stbuf);
attr->mtimensec = ST_MTIM_NSEC(stbuf);
attr->ctimensec = ST_CTIM_NSEC(stbuf);
+
+   if (stbuf->st_size >= 1048576)
+   attr->flags |= FUSE_ATTR_DAX;
 }
 
 static void convert_attr(const struct fuse_setattr_in *attr, struct stat 
*stbuf)
-- 
2.27.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH 3/3] fuse: add per-file DAX flag

2021-07-15 Thread Vivek Goyal 
On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
> 
> 
> On 7/16/21 8:51 AM, Vivek Goyal wrote:
> > On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> >> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> >>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> >>> this file.
> >>>
> >>> When the per-file DAX flag changes for an *opened* file, the state of
> >>> the file won't be updated until this file is closed and reopened later.
> >>>
> >>> Currently it is not implemented yet to change per-file DAX flag inside
> >>> guest kernel, e.g., by chattr(1).
> >>
> >> Thanks for the patch, it looks good to me.
> >>
> >> I think it's a good starting point, what I'd like to discuss here is
> >> whether we're going to let chattr to toggle the dax flag.
> > 
> > I have the same question. Why not take chattr approach as taken
> > by ext4/xfs as well.
> > 
> > Vivek
> 
> Thanks.
> 
> We can implement the chattr approach as ext4/xfs do, if we have this use
> scenario. It's an RFC patch, and I want to collect more feedback as soon
> as possible.

I guess chattr approach will allow client (as well as server) to control
which files should be DAX. While this approach allows only server to
specify which files should use DAX. Given currently we let client
control whether to use dax or not (-o dax), it probably will make
sense to use chattr based approach?

I will look at the patches. Do you have a corresponding user space
implementation somewhere so that I can test it?

Vivek

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH 3/3] fuse: add per-file DAX flag

2021-07-15 Thread JeffleXu 



On 7/16/21 8:51 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
>> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
>>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
>>> this file.
>>>
>>> When the per-file DAX flag changes for an *opened* file, the state of
>>> the file won't be updated until this file is closed and reopened later.
>>>
>>> Currently it is not implemented yet to change per-file DAX flag inside
>>> guest kernel, e.g., by chattr(1).
>>
>> Thanks for the patch, it looks good to me.
>>
>> I think it's a good starting point, what I'd like to discuss here is
>> whether we're going to let chattr to toggle the dax flag.
> 
> I have the same question. Why not take chattr approach as taken
> by ext4/xfs as well.
> 
> Vivek

Thanks.

We can implement the chattr approach as ext4/xfs do, if we have this use
scenario. It's an RFC patch, and I want to collect more feedback as soon
as possible.

-- 
Thanks,
Jeffle
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block

2021-07-15 Thread kernel test robot 
Hi Yury,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on vhost/linux-next hch-configfs/for-next linus/master 
v5.14-rc1 next-20210715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Yury-Kamenev/virtio-disable-partitions-scanning-for-no-partitions-block/20210715-175107
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git 
for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/b5b35e33f22266b3905186a005992d54ae71e51b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Yury-Kamenev/virtio-disable-partitions-scanning-for-no-partitions-block/20210715-175107
git checkout b5b35e33f22266b3905186a005992d54ae71e51b
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross 
O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/block/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/spinlock.h:51,
from drivers/block/virtio_blk.c:3:
   drivers/block/virtio_blk.c: In function 'virtblk_probe':
>> drivers/block/virtio_blk.c:807:15: error: 'partitions_scanning_disable' 
>> undeclared (first use in this function)
 807 |  if (unlikely(partitions_scanning_disable))
 |   ^~~
   include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
  78 | # define unlikely(x) __builtin_expect(!!(x), 0)
 |  ^
   drivers/block/virtio_blk.c:807:15: note: each undeclared identifier is 
reported only once for each function it appears in
 807 |  if (unlikely(partitions_scanning_disable))
 |   ^~~
   include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
  78 | # define unlikely(x) __builtin_expect(!!(x), 0)
 |  ^


vim +/partitions_scanning_disable +807 drivers/block/virtio_blk.c

   707  
   708  static int virtblk_probe(struct virtio_device *vdev)
   709  {
   710  struct virtio_blk *vblk;
   711  struct request_queue *q;
   712  int err, index;
   713  
   714  u32 v, blk_size, max_size, sg_elems, opt_io_size;
   715  u16 min_io_size;
   716  u8 physical_block_exp, alignment_offset;
   717  unsigned int queue_depth;
   718  
   719  if (!vdev->config->get) {
   720  dev_err(>dev, "%s failure: config access 
disabled\n",
   721  __func__);
   722  return -EINVAL;
   723  }
   724  
   725  err = ida_simple_get(_index_ida, 0, minor_to_index(1 << 
MINORBITS),
   726   GFP_KERNEL);
   727  if (err < 0)
   728  goto out;
   729  index = err;
   730  
   731  /* We need to know how many segments before we allocate. */
   732  err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
   733 struct virtio_blk_config, seg_max,
   734 _elems);
   735  
   736  /* We need at least one SG element, whatever they say. */
   737  if (err || !sg_elems)
   738  sg_elems = 1;
   739  
   740  /* Prevent integer overflows and honor max vq size */
   741  sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
   742  
   743  /* We need extra sg elements at head and tail. */
   744  sg_elems += 2;
   745  vdev->priv = vblk = kmalloc(sizeof(*vblk), GFP_KERNEL);
   746  if (!vblk) {
   747  err = -ENOMEM;
   748  goto out_free_index;
   749  }
   750  
   751  /* This reference is dropped in virtblk_remove(). */
   752  refcount_set(>refs, 1);
   753  mutex_init(>vdev_mutex);
   754  
   755  vblk->vdev = vdev;
   756  vblk->sg_elems = sg_elems;
   757  
   758  INIT_WORK(>config_work, virtblk_config_changed_work);
   759  
   7

Re: [RFC PATCH 3/3] fuse: add per-file DAX flag

2021-07-15 Thread Vivek Goyal 
On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> > Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> > this file.
> > 
> > When the per-file DAX flag changes for an *opened* file, the state of
> > the file won't be updated until this file is closed and reopened later.
> > 
> > Currently it is not implemented yet to change per-file DAX flag inside
> > guest kernel, e.g., by chattr(1).
> 
> Thanks for the patch, it looks good to me.
> 
> I think it's a good starting point, what I'd like to discuss here is
> whether we're going to let chattr to toggle the dax flag.

I have the same question. Why not take chattr approach as taken
by ext4/xfs as well.

Vivek

> 
> My usecase is like, on the fuse server side, if a file is marked as
> DAX, then it won't change any more.  So this 'fuse_attr.flags' works
> for me at least.
> 
> thanks,
> liubo
> 
> > 
> > Signed-off-by: Jeffle Xu 
> > ---
> >  fs/fuse/dax.c | 28 
> >  fs/fuse/file.c|  4 ++--
> >  fs/fuse/fuse_i.h  |  5 +++--
> >  fs/fuse/inode.c   |  4 +++-
> >  include/uapi/linux/fuse.h |  5 +
> >  5 files changed, 37 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > index 4873d764cb66..ed5a430364bb 100644
> > --- a/fs/fuse/dax.c
> > +++ b/fs/fuse/dax.c
> > @@ -1341,7 +1341,7 @@ static const struct address_space_operations 
> > fuse_dax_file_aops  = {
> > .invalidatepage = noop_invalidatepage,
> >  };
> >  
> > -static bool fuse_should_enable_dax(struct inode *inode)
> > +static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
> >  {
> > struct fuse_conn *fc = get_fuse_conn(inode);
> > unsigned int mode;
> > @@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode 
> > *inode)
> > if (mode == FUSE_DAX_MOUNT_NEVER)
> > return false;
> >  
> > -   return true;
> > +   if (mode == FUSE_DAX_MOUNT_ALWAYS)
> > +   return true;
> > +
> > +   WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
> > +   return flags & FUSE_ATTR_DAX;
> >  }
> >  
> > -void fuse_dax_inode_init(struct inode *inode)
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
> >  {
> > -   if (!fuse_should_enable_dax(inode))
> > +   if (!fuse_should_enable_dax(inode, flags))
> > return;
> >  
> > inode->i_flags |= S_DAX;
> > inode->i_data.a_ops = _dax_file_aops;
> >  }
> >  
> > +void fuse_update_dax(struct inode *inode, unsigned int flags)
> > +{
> > +   bool oldstate, newstate;
> > +   struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > +   if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
> > +   fc->dax->mode != FUSE_DAX_MOUNT_INODE)
> > +   return;
> > +
> > +   oldstate = IS_DAX(inode);
> > +   newstate = flags & FUSE_ATTR_DAX;
> > +
> > +   if (oldstate != newstate)
> > +   d_mark_dontcache(inode);
> > +}
> > +
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int 
> > map_alignment)
> >  {
> > if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 97f860cfc195..cf42af492146 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3142,7 +3142,7 @@ static const struct address_space_operations 
> > fuse_file_aops  = {
> > .write_end  = fuse_write_end,
> >  };
> >  
> > -void fuse_init_file_inode(struct inode *inode)
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
> >  {
> > struct fuse_inode *fi = get_fuse_inode(inode);
> >  
> > @@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
> > fi->writepages = RB_ROOT;
> >  
> > if (IS_ENABLED(CONFIG_FUSE_DAX))
> > -   fuse_dax_inode_init(inode);
> > +   fuse_dax_inode_init(inode, attr->flags);
> >  }
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index f29018323845..b0ecfffd0c7d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
> >  /**
> >   * Initialize file operations on a regular file
> >   */
> > -void fuse_init_file_inode(struct inode *inode);
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
> >  
> >  /**
> >   * Initialize inode operations on regular files and special files
> > @@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, 
> > unsigned int mode,
> > struct dax_device *dax_dev);
> >  void fuse_dax_conn_free(struct fuse_conn *fc);
> >  bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
> > -void fuse_dax_inode_init(struct inode *inode);
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
> >  void fuse_dax_inode_cleanup(struct inode *inode);
> > +void fuse_update_dax(struct inode *inode, unsigned int flags);
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc,

Re: [PATCH v1 1/4] mm/memory_hotplug: use "unsigned long" for PFN in zone_for_pfn_range()

2021-07-15 Thread Andrew Morton 
On Thu, 15 Jul 2021 11:42:21 +0200 David Hildenbrand  wrote:

> > I'd propose to add Cc:  since I actually had
> > the fun to try to debug something like this a couple of years ago:
> > 6cdb18ad98a4 ("mm/vmstat: fix overflow in mod_zone_page_state()")
> > 
> 
> Good point, and thinking again what can go wrong, I tend to agree. We 
> are trying to keep zones contiguous and it could happen that we end up 
> with something like ZONE_DMA here (via default_kernel_zone_for_pfn()) 
> and would consequently online something to ZONE_DMA that doesn't belong 
> there, resulting in crashes.
> 
> @Andrew can you add  Cc:  and
> 
> "As we will search for a fitting zone using the wrong pfn, we might end 
> up onlining memory to one of the special kernel zones, such as ZONE_DMA, 
> which can end badly as the onlined memory does not satisfy properties of 
> these zones."

Yep, all done.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH -next v2] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()

2021-07-15 Thread Thomas Zimmermann 

Hi

Am 15.07.21 um 13:03 schrieb Yang Yingliang:


On 2021/7/15 17:30, Thomas Zimmermann wrote:

Hi,

for the change


Acked-by: Thomas Zimmermann 


but there are some style issues AFAICS.

OK, need I resend it with the style changes and your ack ?


Please do. I'll merge it a few days later if no further comments come in.

Best regards
Thomas



Am 15.07.21 um 04:05 schrieb Yang Yingliang:

Replace pci_enable_device() with pcim_enable_device(),
pci_disable_device() will be called in release automatically.

Reported-by: Hulk Robot 
Signed-off-by: Yang Yingliang 


S-o-b line goes first


---
v2:
   use pcim_enable_device()


This changelog should rather be located between the commit description 
and the first S-o-b line.


Best regards
Thomas


---
  drivers/gpu/drm/tiny/bochs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index a2cfecfa8556..73415fa9ae0f 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -648,7 +648,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, 
const struct pci_device_id *ent

  if (IS_ERR(dev))
  return PTR_ERR(dev);
  -    ret = pci_enable_device(pdev);
+    ret = pcim_enable_device(pdev);
  if (ret)
  goto err_free_dev;





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block

2021-07-15 Thread Paolo Bonzini 

On 15/07/21 11:47, Yury Kamenev wrote:

+#ifdef CONFIG_VIRTIO_BLK_NO_PART_SCAN
+   if (unlikely(partitions_scanning_disable))
+   /* disable partitions scanning if it was stated in virtio 
features*/
+   if (virtio_has_feature(vdev, VIRTIO_BLK_F_NO_PART_SCAN))
+   vblk->disk->flags |= GENHD_FL_NO_PART_SCAN;
+#endif
+


Has this been added to the spec?  It doesn't seem like a good idea, as 
pointed out by Stefan[1], Christoph[2] and myself[3].


Paolo

[1] 
https://lore.kernel.org/linux-block/20210524145654.ga2...@lst.de/T/#m2697cb41578490aad49ed1d8fa6604bf0924b54d
[2] 
https://lore.kernel.org/linux-block/20210524145654.ga2...@lst.de/T/#mc59329fd824102f94ac2f6b29fe94a652849aca0
[3] 
https://lore.kernel.org/linux-block/20210524145654.ga2...@lst.de/T/#mee6787c4fd87790b64feccc9e77fd5f618c2c336


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1 1/4] mm/memory_hotplug: use "unsigned long" for PFN in zone_for_pfn_range()

2021-07-15 Thread David Hildenbrand 

On 14.07.21 22:13, Heiko Carstens wrote:

On Mon, Jul 12, 2021 at 02:40:49PM +0200, David Hildenbrand wrote:

Checkpatch complained on a follow-up patch that we are using "unsigned"
here, which defaults to "unsigned int" and checkpatch is correct.

Use "unsigned long" instead, just as we do in other places when handling
PFNs. This can bite us once we have physical addresses in the range of
multiple TB.

Fixes: e5e689302633 ("mm, memory_hotplug: display allowed zones in the preferred 
ordering")
Signed-off-by: David Hildenbrand 
---
  include/linux/memory_hotplug.h | 4 ++--
  mm/memory_hotplug.c| 4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)


I'd propose to add Cc:  since I actually had
the fun to try to debug something like this a couple of years ago:
6cdb18ad98a4 ("mm/vmstat: fix overflow in mod_zone_page_state()")



Good point, and thinking again what can go wrong, I tend to agree. We 
are trying to keep zones contiguous and it could happen that we end up 
with something like ZONE_DMA here (via default_kernel_zone_for_pfn()) 
and would consequently online something to ZONE_DMA that doesn't belong 
there, resulting in crashes.


@Andrew can you add  Cc:  and

"As we will search for a fitting zone using the wrong pfn, we might end 
up onlining memory to one of the special kernel zones, such as ZONE_DMA, 
which can end badly as the onlined memory does not satisfy properties of 
these zones."


Thanks Heiko!

--
Thanks,

David / dhildenb

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH -next v2] drm/bochs: Fix missing pci_disable_device() on error in bochs_pci_probe()

2021-07-15 Thread Thomas Zimmermann 

Hi,

for the change


Acked-by: Thomas Zimmermann 


but there are some style issues AFAICS.

Am 15.07.21 um 04:05 schrieb Yang Yingliang:

Replace pci_enable_device() with pcim_enable_device(),
pci_disable_device() will be called in release automatically.

Reported-by: Hulk Robot 
Signed-off-by: Yang Yingliang 


S-o-b line goes first


---
v2:
   use pcim_enable_device()


This changelog should rather be located between the commit description 
and the first S-o-b line.


Best regards
Thomas


---
  drivers/gpu/drm/tiny/bochs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index a2cfecfa8556..73415fa9ae0f 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -648,7 +648,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent
if (IS_ERR(dev))
return PTR_ERR(dev);
  
-	ret = pci_enable_device(pdev);

+   ret = pcim_enable_device(pdev);
if (ret)
goto err_free_dev;
  



--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[RFC PATCH 2/3] fuse: Make DAX mount option a tri-state

2021-07-15 Thread Jeffle Xu 
We add 'always', 'never', and 'inode' (default). '-o dax' continues to
operate the same which is equivalent to 'always'.

By the time this patch is applied, 'inode' mode is actually equal to
'always' mode, before the per-file DAX flag is introduced in the
following patch.

Signed-off-by: Jeffle Xu 
---
 fs/fuse/dax.c   | 13 -
 fs/fuse/fuse_i.h| 11 +--
 fs/fuse/inode.c |  2 +-
 fs/fuse/virtio_fs.c | 16 ++--
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 97b8bd09baa3..4873d764cb66 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -70,6 +70,9 @@ struct fuse_inode_dax {
 };
 
 struct fuse_conn_dax {
+   /** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/
+   unsigned int mode;
+
/* DAX device */
struct dax_device *dev;
 
@@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax 
*fcd)
return ret;
 }
 
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+   struct dax_device *dax_dev)
 {
struct fuse_conn_dax *fcd;
int err;
@@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct 
dax_device *dax_dev)
return -ENOMEM;
 
spin_lock_init(>lock);
+   fcd->mode = mode;
fcd->dev = dax_dev;
err = fuse_dax_mem_range_init(fcd);
if (err) {
@@ -1339,10 +1344,16 @@ static const struct address_space_operations 
fuse_dax_file_aops  = {
 static bool fuse_should_enable_dax(struct inode *inode)
 {
struct fuse_conn *fc = get_fuse_conn(inode);
+   unsigned int mode;
 
if (!fc->dax)
return false;
  
+   mode = fc->dax->mode;
+
+   if (mode == FUSE_DAX_MOUNT_NEVER)
+   return false;
+
return true;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 07829ce78695..f29018323845 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -487,6 +487,12 @@ struct fuse_dev {
struct list_head entry;
 };
 
+enum {
+   FUSE_DAX_MOUNT_INODE,
+   FUSE_DAX_MOUNT_ALWAYS,
+   FUSE_DAX_MOUNT_NEVER,
+};
+
 struct fuse_fs_context {
int fd;
unsigned int rootmode;
@@ -503,7 +509,7 @@ struct fuse_fs_context {
bool no_control:1;
bool no_force_umount:1;
bool legacy_opts_show:1;
-   bool dax:1;
+   unsigned int dax;
unsigned int max_read;
unsigned int blksize;
const char *subtype;
@@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct 
iov_iter *to);
 ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from);
 int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma);
 int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end);
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev);
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+   struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
 void fuse_dax_inode_init(struct inode *inode);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b9beb39a4a18..f6b46395edb2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct 
fuse_fs_context *ctx)
sb->s_subtype = ctx->subtype;
ctx->subtype = NULL;
if (IS_ENABLED(CONFIG_FUSE_DAX)) {
-   err = fuse_dax_conn_alloc(fc, ctx->dax_dev);
+   err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev);
if (err)
goto err;
}
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 8f52cdaa8445..561f711d1945 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -88,12 +88,21 @@ struct virtio_fs_req_work {
 static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
 struct fuse_req *req, bool in_flight);
 
+static const struct constant_table dax_param_enums[] = {
+   {"inode",   FUSE_DAX_MOUNT_INODE },
+   {"always",  FUSE_DAX_MOUNT_ALWAYS },
+   {"never",   FUSE_DAX_MOUNT_NEVER },
+   {}
+};
+
 enum {
OPT_DAX,
+   OPT_DAX_ENUM,
 };
 
 static const struct fs_parameter_spec virtio_fs_parameters[] = {
fsparam_flag("dax", OPT_DAX),
+   fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums),
{}
 };
 
@@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc,
 
switch (opt) {
case OPT_DAX:
-   ctx->dax = 1;
+   ctx->dax = FUSE_DAX_MOUNT_ALWAYS;
+   break;
+   case OPT_DAX_ENUM:
+   ctx->dax = result.uint_32;
break;
default:
return -EINVAL;
@@ -1326,7 +1338,7 @@ static int

[RFC PATCH 1/3] fuse: add fuse_should_enable_dax() helper

2021-07-15 Thread Jeffle Xu 
This is in prep for following per-file DAX checking.

Signed-off-by: Jeffle Xu 
---
 fs/fuse/dax.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 0e5407f48e6a..97b8bd09baa3 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1336,11 +1336,19 @@ static const struct address_space_operations 
fuse_dax_file_aops  = {
.invalidatepage = noop_invalidatepage,
 };
 
-void fuse_dax_inode_init(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode)
 {
struct fuse_conn *fc = get_fuse_conn(inode);
 
if (!fc->dax)
+   return false;
+ 
+   return true;
+}
+
+void fuse_dax_inode_init(struct inode *inode)
+{
+   if (!fuse_should_enable_dax(inode))
return;
 
inode->i_flags |= S_DAX;
-- 
2.27.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[RFC PATCH 3/3] fuse: add per-file DAX flag

2021-07-15 Thread Jeffle Xu 
Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
this file.

When the per-file DAX flag changes for an *opened* file, the state of
the file won't be updated until this file is closed and reopened later.

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Signed-off-by: Jeffle Xu 
---
 fs/fuse/dax.c | 28 
 fs/fuse/file.c|  4 ++--
 fs/fuse/fuse_i.h  |  5 +++--
 fs/fuse/inode.c   |  4 +++-
 include/uapi/linux/fuse.h |  5 +
 5 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 4873d764cb66..ed5a430364bb 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1341,7 +1341,7 @@ static const struct address_space_operations 
fuse_dax_file_aops  = {
.invalidatepage = noop_invalidatepage,
 };
 
-static bool fuse_should_enable_dax(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
 {
struct fuse_conn *fc = get_fuse_conn(inode);
unsigned int mode;
@@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
if (mode == FUSE_DAX_MOUNT_NEVER)
return false;
 
-   return true;
+   if (mode == FUSE_DAX_MOUNT_ALWAYS)
+   return true;
+
+   WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
+   return flags & FUSE_ATTR_DAX;
 }
 
-void fuse_dax_inode_init(struct inode *inode)
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
 {
-   if (!fuse_should_enable_dax(inode))
+   if (!fuse_should_enable_dax(inode, flags))
return;
 
inode->i_flags |= S_DAX;
inode->i_data.a_ops = _dax_file_aops;
 }
 
+void fuse_update_dax(struct inode *inode, unsigned int flags)
+{
+   bool oldstate, newstate;
+   struct fuse_conn *fc = get_fuse_conn(inode);
+
+   if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
+   fc->dax->mode != FUSE_DAX_MOUNT_INODE)
+   return;
+
+   oldstate = IS_DAX(inode);
+   newstate = flags & FUSE_ATTR_DAX;
+
+   if (oldstate != newstate)
+   d_mark_dontcache(inode);
+}
+
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
 {
if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 97f860cfc195..cf42af492146 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3142,7 +3142,7 @@ static const struct address_space_operations 
fuse_file_aops  = {
.write_end  = fuse_write_end,
 };
 
-void fuse_init_file_inode(struct inode *inode)
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
 {
struct fuse_inode *fi = get_fuse_inode(inode);
 
@@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
fi->writepages = RB_ROOT;
 
if (IS_ENABLED(CONFIG_FUSE_DAX))
-   fuse_dax_inode_init(inode);
+   fuse_dax_inode_init(inode, attr->flags);
 }
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f29018323845..b0ecfffd0c7d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
 /**
  * Initialize file operations on a regular file
  */
-void fuse_init_file_inode(struct inode *inode);
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
 
 /**
  * Initialize inode operations on regular files and special files
@@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned 
int mode,
struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
-void fuse_dax_inode_init(struct inode *inode);
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
 void fuse_dax_inode_cleanup(struct inode *inode);
+void fuse_update_dax(struct inode *inode, unsigned int flags);
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int 
map_alignment);
 void fuse_dax_cancel_work(struct fuse_conn *fc);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f6b46395edb2..47ebb1a394d2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct 
fuse_attr *attr,
if (inval)
invalidate_inode_pages2(inode->i_mapping);
}
+
+   fuse_update_dax(inode, attr->flags);
 }
 
 static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
@@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct 
fuse_attr *attr)
inode->i_ctime.tv_nsec = attr->ctimensec;
if (S_ISREG(inode->i_mode)) {
fuse_init_common(inode);
-   fuse_init_file_inode(inode);
+   fuse_init_file_inode(inode, attr);
} else if (S_ISDIR(inode->i_mode))

[RFC PATCH 0/3] virtiofs,fuse: support per-file DAX

2021-07-15 Thread Jeffle Xu 
This patchset adds support of per-file DAX for virtiofs, which is
inspired by Ira Weiny's work on ext4[1] and xfs[2].

Currently virtiofs (in guest kernel) accepts per-file DAX flag from FUSE
server (in host). 

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Any comment is welcome. :)


[1] commit 9cb20f94afcd ("fs/ext4: Make DAX mount option a tri-state")
[2] commit 02beb2686ff9 ("fs/xfs: Make DAX mount option a tri-state")

Jeffle Xu (3):
  fuse: add fuse_should_enable_dax() helper
  fuse: Make DAX mount option a tri-state
  fuse: add per-file DAX flag

 fs/fuse/dax.c | 43 +--
 fs/fuse/file.c|  4 ++--
 fs/fuse/fuse_i.h  | 16 +++
 fs/fuse/inode.c   |  6 --
 fs/fuse/virtio_fs.c   | 16 +--
 include/uapi/linux/fuse.h |  5 +
 6 files changed, 78 insertions(+), 12 deletions(-)

-- 
2.27.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 4/4] vdpa: Add documentation for vdpa_alloc_device() macro

2021-07-15 Thread Jason Wang 


在 2021/7/15 下午4:00, Xie Yongji 写道:

The return value of vdpa_alloc_device() macro is not very
clear, so that most of callers did the wrong check. Let's
add some comments to better document it.

Signed-off-by: Xie Yongji 



Acked-by: Jason Wang 



---
  include/linux/vdpa.h | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 3357ac98878d..8cfe49d201dd 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -277,6 +277,17 @@ struct vdpa_device *__vdpa_alloc_device(struct device 
*parent,
const struct vdpa_config_ops *config,
size_t size, const char *name);
  
+/**

+ * vdpa_alloc_device - allocate and initilaize a vDPA device
+ *
+ * @dev_struct: the type of the parent structure
+ * @member: the name of struct vdpa_device within the @dev_struct
+ * @parent: the parent device
+ * @config: the bus operations that is supported by this device
+ * @name: name of the vdpa device
+ *
+ * Return allocated data structure or ERR_PTR upon error
+ */
  #define vdpa_alloc_device(dev_struct, member, parent, config, name)   \
  container_of(__vdpa_alloc_device( \
   parent, config, \


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/4] vDPA/ifcvf: Fix return value check for vdpa_alloc_device()

2021-07-15 Thread Jason Wang 


在 2021/7/15 下午4:00, Xie Yongji 写道:

The vdpa_alloc_device() returns an error pointer upon
failure, not NULL. To handle the failure correctly, this
replaces NULL check with IS_ERR() check and propagate the
error upwards.

Fixes: 5a2414bc454e ("virtio: Intel IFC VF driver for VDPA")
Reported-by: Dan Carpenter 
Signed-off-by: Xie Yongji 



Acked-by: Jason Wang 



---
  drivers/vdpa/ifcvf/ifcvf_main.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 21b78f1cd521..351c6cfb24c3 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -493,9 +493,9 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
  
  	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,

dev, _vdpa_ops, NULL);
-   if (adapter == NULL) {
+   if (IS_ERR(adapter)) {
IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
-   return -ENOMEM;
+   return PTR_ERR(adapter);
}
  
  	pci_set_master(pdev);


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/4] vp_vdpa: Fix return value check for vdpa_alloc_device()

2021-07-15 Thread Jason Wang 


在 2021/7/15 下午4:00, Xie Yongji 写道:

The vdpa_alloc_device() returns an error pointer upon
failure, not NULL. To handle the failure correctly, this
replaces NULL check with IS_ERR() check and propagate the
error upwards.

Fixes: 64b9f64f80a6 ("vdpa: introduce virtio pci driver")
Reported-by: Dan Carpenter 
Signed-off-by: Xie Yongji 



Acked-by: Jason Wang 



---
  drivers/vdpa/virtio_pci/vp_vdpa.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c 
b/drivers/vdpa/virtio_pci/vp_vdpa.c
index 7b4a6396c553..fe0527329857 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -436,9 +436,9 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
  
  	vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,

dev, _vdpa_ops, NULL);
-   if (vp_vdpa == NULL) {
+   if (IS_ERR(vp_vdpa)) {
dev_err(dev, "vp_vdpa: Failed to allocate vDPA structure\n");
-   return -ENOMEM;
+   return PTR_ERR(vp_vdpa);
}
  
  	mdev = _vdpa->mdev;


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/4] vdpa_sim: Fix return value check for vdpa_alloc_device()

2021-07-15 Thread Jason Wang 


在 2021/7/15 下午3:59, Xie Yongji 写道:

The vdpa_alloc_device() returns an error pointer upon
failure, not NULL. To handle the failure correctly, this
replaces NULL check with IS_ERR() check and propagate the
error upwards.

Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Reported-by: Dan Carpenter 
Signed-off-by: Xie Yongji 



Acked-by: Jason Wang 



---
  drivers/vdpa/vdpa_sim/vdpa_sim.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 14e024de5cbf..c621cf7feec0 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -251,8 +251,10 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr 
*dev_attr)
  
  	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,

dev_attr->name);
-   if (!vdpasim)
+   if (IS_ERR(vdpasim)) {
+   ret = PTR_ERR(vdpasim);
goto err_alloc;
+   }
  
  	vdpasim->dev_attr = *dev_attr;

INIT_WORK(>work, dev_attr->work_fn);


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization