[PATCH] ocfs2: free 'dummy_sc' in sc_fop_release() in case of memory leak

'sd->dbg_sock' is malloc in sc_common_open(), but not freed at the end of sc_fop_release(). Signed-off-by: Jun Piao --- fs/ocfs2/cluster/netdebug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/ocfs2/cluster/netdebug.c b/fs/ocfs2/cluster/netdebug.c index

Re: [Ocfs2-devel] [PATCH v2 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

On 2017/11/29 16:36, Gang He wrote: > Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which > will be used in non-block IO scenarios. > > Signed-off-by: Gang He Reviewed-by: Jun Piao > --- > fs/ocfs2/dlmglue.c | 21 + >

Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

Hi Gang, On 2017/11/27 17:46, Gang He wrote: > Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which > will be used in non-block IO scenarios. > > Signed-off-by: Gang He > --- > fs/ocfs2/dlmglue.c | 22 ++ > fs/ocfs2/dlmglue.h | 4 > 2 files

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

Hi Gang, If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if we can discard 'int wait' just as ext4 does: static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len); thans, Jun On 2017/11/27 17:46, Gang He wrote: > Add ocfs2_overwrite_io function, which is

Re: [Ocfs2-devel] [PATCH v2] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

LGTM On 2017/12/28 15:48, Gang He wrote: > If we can't get inode lock immediately in the function > ocfs2_inode_lock_with_page() when reading a page, we should not > return directly here, since this will lead to a softlockup problem > when the kernel is configured with CONFIG_PREEMPT is not set.

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, Do you mean that too many retrys in loop cast losts of CPU-time and block page-fault interrupt? We should not add any delay in ocfs2_fault(), right? And I still feel a little confused why your method can solve this problem. thanks, Jun On 2017/12/27 17:29, Gang He wrote: > If we can't

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, Thanks for your explaination, and I just have one more question. Could we use 'ocfs2_inode_lock' instead of 'ocfs2_inode_lock_full' to avoid -EAGAIN circularly? thanks, Jun On 2017/12/27 18:37, Gang He wrote: > Hi Jun, > > >> Hi Gang, >> >> Do you mean that too many retrys in

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, You cleared my doubt. Should we handle the errno of ocfs2_inode_lock() or just use mlog_errno()? thanks, Jun On 2017/12/28 10:11, Gang He wrote: > Hi Jun, > > >> Hi Gang, >> >> Thanks for your explaination, and I just have one more question. Could >> we use 'ocfs2_inode_lock'

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, This patch looks good to me. thanks, Jun On 2017/12/28 10:58, Gang He wrote: > > > >> Hi Gang, >> >> You cleared my doubt. Should we handle the errno of ocfs2_inode_lock() >> or just use mlog_errno()? > Hi Jun, I think it is not necessary, since we just want to hold a while >

Re: [V9fs-developer] [PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

our suggestion for the next version of the patch, or this patch is good enough? Thanks, Jun On 2018/8/2 21:23, Greg Kurz wrote: > On Thu, 2 Aug 2018 09:59:38 +0800 > piaojun wrote: > >> Hi Dominique, >> >> On 2018/8/2 9:54, Dominique Martinet wrote: >>> piaojun wrote

[PATCH v3] net/9p/trans_virtio.c: add null terminal for mount tag

chan->tag is Non-null terminated which will result in printing messy code when debugging code. So we should add '\0' for tag to make the code more convenient and robust. In addition, I drop char->tag_len to simplify the code. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 17

Re: [V9fs-developer] [PATCH 2/2] net/9p: add a per-client fcall kmem_cache

On 2018/7/31 9:35, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 31, 2018: >> Could you help paste some test result before-and-after the patch applied? > > The only performance tests I did were sent to the list a couple of mails > earlier, you can find it here: > h

Re: [V9fs-developer] [PATCH 2/2] net/9p: add a per-client fcall kmem_cache

Hi Dominique, Could you help paste some test result before-and-after the patch applied? And I have a little suggestion in comments below. On 2018/7/30 17:34, Dominique Martinet wrote: > From: Dominique Martinet > > Having a specific cache for the fcall allocations helps speed up > allocations

Re: [V9fs-developer] [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs

On 2018/7/31 9:12, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 31, 2018: >> This is really a *big* patch, but the modification seems no harm. And I >> suggest running testcases to cover this. Please see my comments below. > > I'm always running tests, but mor

Re: [V9fs-developer] [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs

Hi Dominique, This is really a *big* patch, but the modification seems no harm. And I suggest running testcases to cover this. Please see my comments below. On 2018/7/30 17:34, Dominique Martinet wrote: > From: Dominique Martinet > > 'msize' is often a power of two, or at least page-aligned,

Re: [PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

Hi Dominique, On 2018/8/1 16:11, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 01, 2018: >> chan->tag has no terminal char at last which will result in printing messy >> code when debugging code. So we should add '\0' for tag. > > 9p is full of non null-terminate

[PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

chan->tag has no terminal char at last which will result in printing messy code when debugging code. So we should add '\0' for tag. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c

Re: [PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

Hi Dominique and Greg, Thanks for your reviewing, and I will try to simplify other related code according your suggestions in patch v2. Thanks, Jun On 2018/8/1 20:09, Dominique Martinet wrote: > Greg Kurz wrote on Wed, Aug 01, 2018: >> So this patch basically turns chan->tag into a nul

[PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

chan->tag is Non-null terminated which will result in printing messy code when debugging code. So we should add '\0' for tag to make the code more convenient and robust. In addition, I drop char->tag_len to simplify the code. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 15

Re: [PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

Hi Dominique, On 2018/8/2 9:54, Dominique Martinet wrote: > piaojun wrote on Thu, Aug 02, 2018: >> chan->tag is Non-null terminated which will result in printing messy code >> when debugging code. So we should add '\0' for tag to make the code more >> convenient and rob

Re: [PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

Hi Dominique, On 2018/8/8 17:40, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 08, 2018: >> I try to remove 9pnet_virtio.ko by 'rmmod 9pnet_virtio' as I want to >> replace it without rebooting system. > > I do that all the time when testing, it works for me. >

Re: [PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

Hi Dominique, On 2018/8/8 16:36, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 08, 2018: >> I found that 9pnet_virtio.ko could not be removed by rmmod command, and I >> could still found it by lsmod. The reason is that we forgot decrease the >> refcount of 9p virtio

[PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

I found that 9pnet_virtio.ko could not be removed by rmmod command, and I could still found it by lsmod. The reason is that we forgot decrease the refcount of 9p virtio device by kobject_put. So we should put refcount in p9_virtio_remove. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 3

Re: [PATCH v3 2/2] net/9p: add a per-client fcall kmem_cache

Hi Dominique, Could you help paste the test result of before-after-applied this patch in comment? And please see my comments below. On 2018/8/9 22:33, Dominique Martinet wrote: > From: Dominique Martinet > > Having a specific cache for the fcall allocations helps speed up > allocations a bit,

Re: [PATCH v3 1/2] net/9p: embed fcall in req to round down buffer allocs

LGTM On 2018/8/9 22:33, Dominique Martinet wrote: > From: Dominique Martinet > > 'msize' is often a power of two, or at least page-aligned, so avoiding > an overhead of two dozen bytes for each allocation will help the > allocator do its work and reduce memory fragmentation. > > Suggested-by:

Re: [PATCH v3 2/2] net/9p: add a per-client fcall kmem_cache

Thanks for clearing my doubt, and you can add: Acked-by: Jun Piao On 2018/8/10 9:41, Dominique Martinet wrote: > piaojun wrote on Fri, Aug 10, 2018: >> Could you help paste the test result of before-after-applied this patch in >> comment? And please see my comments bel

[PATCH] scsi/virio_scsi.c: do not call virtscsi_remove_vqs() in virtscsi_init() to avoid crash bug

If some error happened before find_vqs, error branch will goto virtscsi_remove_vqs to free vqs. Actually the vqs have not been allocated successfully, so this will cause wild-pointer-free problem. So virtscsi_remove_vqs could be deleted as no error will happen after find_vqs. Signed-off-by: Jun

Re: [PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

Hi Dominique, On 2018/7/18 17:54, Dominique Martinet wrote: > piaojun wrote on Wed, Jul 18, 2018: >> spin_lock is more effective for short time protection than mutex_lock, as >> mutex lock may cause process sleep and wake up which consume much cpu >> time. > > That's n

Re: [PATCH v2] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

Hi Dominique, On 2018/7/17 19:56, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 17, 2018: >> In p9_read_work(), we use spin_lock for client->lock, but misuse >> spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be >> locked in irq context

[PATCH] net/9p/trans_virtio.c: fix some spell mistakes in comments

Fix spelling mistake in comments of p9_virtio_zc_request(). Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index c9f2717..86077f7 100644 --- a/net/9p/trans_virtio.c +++

[PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

spin_lock is more effective for short time protection than mutex_lock, as mutex lock may cause process sleep and wake up which consume much cpu time. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git

Re: [V9fs-developer] [PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

Hi Dominique, On 2018/7/12 15:01, Dominique Martinet wrote: > piaojun wrote on Thu, Jul 12, 2018: >> In p9_read_work(), we use spin_lock for client->lock, but misuse >> spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be >> locked in irq context

[PATCH v2] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

In p9_read_work(), we use spin_lock for client->lock, but misuse spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be locked in irq context, so spin_lock is enough. And that will improve the performance. Rebased on 9p-next. Signed-off-by: Jun Piao --- net/9p/client.c | 18

[PATCH] fs/9p/xattr.c: catch the error of p9_client_clunk when setting xattr failed

In my testing, v9fs_fid_xattr_set will return successfully even if the backend ext4 filesystem has no space to store xattr key-value. That will cause inconsistent behavior between front end and back end. The reason is that lsetxattr will be triggered by p9_client_clunk, and unfortunately we did

Re: [PATCH] fs/9p/xattr.c: catch the error of p9_client_clunk when setting xattr failed

Hi Dominique, On 2018/7/25 11:32, Dominique Martinet wrote: > piaojun wrote on Wed, Jul 25, 2018: >> In my testing, v9fs_fid_xattr_set will return successfully even if the >> backend ext4 filesystem has no space to store xattr key-value. That will >> cause inconsistent behav

Re: [PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

On 2018/7/19 11:36, Dominique Martinet wrote: > piaojun wrote on Thu, Jul 19, 2018: >>> piaojun wrote on Wed, Jul 18, 2018: >>> That's not a fast path operation, I don't mind changing things but I'd >>> like to understand why - these functions are only ever call

Re: [V9fs-developer] [PATCH v3] Fix a hard lockup case in the virtio transport

LGTM On 2018/7/19 15:17, jiangyiwen wrote: > When client has multiple threads that issue io requests > all the time, and the server has a very good performance, > it may cause cpu is running in the irq context for a long > time because it can check virtqueue has buf in the *while* > loop. > > So

Re: [V9fs-developer] [PATCH v2] net/9p: Fix a deadlock case in the virtio transport

LGTM On 2018/7/17 19:03, jiangyiwen wrote: > When client has multiple threads that issue io requests > all the time, and the server has a very good performance, > it may cause cpu is running in the irq context for a long > time because it can check virtqueue has buf in the *while* > loop. > > So

Re: [V9fs-developer] [PATCH] fs: 9p: Adding new return type vm_fault_t

LGTM On 2018/7/2 23:49, Souptick Joarder wrote: > Use new return type vm_fault_t for page_mkwrite > handler. > > see commit 1c8f422059ae ("mm: change return type to > vm_fault_t") for reference. > > Signed-off-by: Souptick Joarder > Reviewed-by: Matthew Wilcox Acked-by: Jun Piao > --- >

[PATCH] net/9p/client.c: put refcount of trans_mod in error case in parse_opts()

>From my test, the second mount will fail after umounting successfully. The reason is that we put refcount of trans_mod in the correct case rather than the error case in parse_opts() at last. That will cause the refcount decrease to -1, and when we try to get trans_mod again in try_module_get(),

[PATCH] net/9p/client.c: add missing '\n' at the end of p9_debug()

In p9_client_getattr_dotl(), we should add '\n' at the end of printing log. Signed-off-by: Jun Piao --- net/9p/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/client.c b/net/9p/client.c index 5c13431..8bc8b3e 100644 --- a/net/9p/client.c +++ b/net/9p/client.c

[PATCH] 9p/net/protocol.c: return -ENOMEM when kmalloc() failed

We should return -ENOMEM to upper user when kmalloc failed to indicate accurate errno. Signed-off-by: Jun Piao --- net/9p/protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/protocol.c b/net/9p/protocol.c index 931ea00..4a1e1dd 100644 --- a/net/9p/protocol.c

Re: [V9fs-developer] [PATCH v2 2/6] 9p: Change p9_fid_create calling convention

LGTM On 2018/7/12 5:02, Matthew Wilcox wrote: > Return NULL instead of ERR_PTR when we can't allocate a FID. The ENOSPC > return value was getting all the way back to userspace, and that's > confusing for a userspace program which isn't expecting read() to tell it > there's no space left on the

[PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

In p9_read_work(), we use spin_lock for client->lock, but misuse spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be locked in irq context, so spin_lock is enough. And that will improve the performance. Signed-off-by: Jun Piao --- net/9p/client.c | 17 +++--

Re: [Ocfs2-devel] [PATCH] Correct a comment error

Hi Larry, There is the same mistake in ocfs2_reflink_inodes_lock(), could you help fixing them all? thanks, Jun On 2018/2/28 18:17, Larry Chen wrote: > The function ocfs2_double_lock tries to lock the inode with lower > blockid first, not lockid. > > Signed-off-by: Larry Chen

Re: [Ocfs2-devel] [PATCH] Correct a comment error

kid', and found 'lockid' is a concept in dlm level, so ocfs2 level should not be aware of it. thanks, Jun > > Thanks, > Changw2ei > > On 2018/3/1 20:58, piaojun wrote: >> Hi Larry, >> >> There is the same mistake in ocfs2_reflink_inodes_lock(), could you help >>

Re: [Ocfs2-devel] [PATCH] Correct a comment error

Hi Larry, There is the same mistake in ocfs2_reflink_inodes_lock(), could you help fixing them all? thanks, Jun On 2018/2/28 18:17, Larry Chen wrote: > The function ocfs2_double_lock tries to lock the inode with lower > blockid first, not lockid. > > Signed-off-by: Larry Chen > --- >

Re: [Ocfs2-devel] [PATCH] Correct a comment error

kid', and found 'lockid' is a concept in dlm level, so ocfs2 level should not be aware of it. thanks, Jun > > Thanks, > Changw2ei > > On 2018/3/1 20:58, piaojun wrote: >> Hi Larry, >> >> There is the same mistake in ocfs2_reflink_inodes_lock(), could you help >>

Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

Hi Gang, On 2017/11/27 17:46, Gang He wrote: > Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which > will be used in non-block IO scenarios. > > Signed-off-by: Gang He > --- > fs/ocfs2/dlmglue.c | 22 ++ > fs/ocfs2/dlmglue.h | 4 > 2 files changed, 26

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: add ocfs2_overwrite_io function

Hi Gang, If ocfs2_overwrite_io is only called in 'nowait' scenarios, I wonder if we can discard 'int wait' just as ext4 does: static bool ext4_overwrite_io(struct inode *inode, loff_t pos, loff_t len); thans, Jun On 2017/11/27 17:46, Gang He wrote: > Add ocfs2_overwrite_io function, which is

Re: [Ocfs2-devel] [PATCH v2 1/3] ocfs2: add ocfs2_try_rw_lock and ocfs2_try_inode_lock

On 2017/11/29 16:36, Gang He wrote: > Add ocfs2_try_rw_lock and ocfs2_try_inode_lock functions, which > will be used in non-block IO scenarios. > > Signed-off-by: Gang He Reviewed-by: Jun Piao > --- > fs/ocfs2/dlmglue.c | 21 + > fs/ocfs2/dlmglue.h | 4 > 2 files

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, Do you mean that too many retrys in loop cast losts of CPU-time and block page-fault interrupt? We should not add any delay in ocfs2_fault(), right? And I still feel a little confused why your method can solve this problem. thanks, Jun On 2017/12/27 17:29, Gang He wrote: > If we can't

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, Thanks for your explaination, and I just have one more question. Could we use 'ocfs2_inode_lock' instead of 'ocfs2_inode_lock_full' to avoid -EAGAIN circularly? thanks, Jun On 2017/12/27 18:37, Gang He wrote: > Hi Jun, > > >> Hi Gang, >> >> Do you mean that too many retrys in

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, You cleared my doubt. Should we handle the errno of ocfs2_inode_lock() or just use mlog_errno()? thanks, Jun On 2017/12/28 10:11, Gang He wrote: > Hi Jun, > > >> Hi Gang, >> >> Thanks for your explaination, and I just have one more question. Could >> we use 'ocfs2_inode_lock'

Re: [Ocfs2-devel] [PATCH] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

Hi Gang, This patch looks good to me. thanks, Jun On 2017/12/28 10:58, Gang He wrote: > > > >> Hi Gang, >> >> You cleared my doubt. Should we handle the errno of ocfs2_inode_lock() >> or just use mlog_errno()? > Hi Jun, I think it is not necessary, since we just want to hold a while >

Re: [Ocfs2-devel] [PATCH v2] ocfs2: try a blocking lock before return AOP_TRUNCATED_PAGE

LGTM On 2017/12/28 15:48, Gang He wrote: > If we can't get inode lock immediately in the function > ocfs2_inode_lock_with_page() when reading a page, we should not > return directly here, since this will lead to a softlockup problem > when the kernel is configured with CONFIG_PREEMPT is not set.

Re: [Virtio-fs] [PATCH 04/19] virtio: Implement get_shm_region for PCI transport

On 2019/8/22 1:57, Vivek Goyal wrote: > From: Sebastien Boeuf > > On PCI the shm regions are found using capability entries; > find a region by searching for the capability. > > Cc: k...@vger.kernel.org > Signed-off-by: Sebastien Boeuf > Signed-off-by: Dr. David Alan Gilbert >

Re: [Virtio-fs] [PATCH 04/19] virtio: Implement get_shm_region for PCI transport

On 2019/8/26 21:06, Vivek Goyal wrote: > On Mon, Aug 26, 2019 at 09:43:08AM +0800, piaojun wrote: > > [..] >>> +static bool vp_get_shm_region(struct virtio_device *vdev, >>> + struct virtio_shm_region *region, u8 id) >>> +{ >

Re: [Virtio-fs] [PATCH 08/18] virtiofs: Drain all pending requests during ->remove time

On 2019/9/6 3:48, Vivek Goyal wrote: > When device is going away, drain all pending requests. > > Signed-off-by: Vivek Goyal > --- > fs/fuse/virtio_fs.c | 83 - > 1 file changed, 51 insertions(+), 32 deletions(-) > > diff --git

Re: [Virtio-fs] [PATCH 15/18] virtiofs: Make virtio_fs object refcounted

On 2019/9/6 3:48, Vivek Goyal wrote: > This object is used both by fuse_connection as well virt device. So make > this object reference counted and that makes it easy to define life cycle > of the object. > > Now deivce can be removed while filesystem is still mounted. This will > cleanup all

Re: [Virtio-fs] [PATCH 16/18] virtiofs: Use virtio_fs_mutex for races w.r.t ->remove and mount path

On 2019/9/6 3:48, Vivek Goyal wrote: > It is possible that a mount is in progress and device is being removed at > the same time. Use virtio_fs_mutex to avoid races. > > This also takes care of bunch of races and removes some TODO items. > > Signed-off-by: Vivek Goyal > --- >

Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On 2019/9/6 19:52, Miklos Szeredi wrote: > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi wrote: >> >> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote: >>> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal wrote: Hi, Michael Tsirkin pointed out issues w.r.t

Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On 2019/9/10 0:14, Stefan Hajnoczi wrote: > On Sun, Sep 08, 2019 at 07:53:55PM +0800, piaojun wrote: >> >> >> On 2019/9/6 19:52, Miklos Szeredi wrote: >>> On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi wrote: >>>> >>>> On Fri,

[PATCH] net/9p/client.c: add missing '\n' at the end of p9_debug()

In p9_client_getattr_dotl(), we should add '\n' at the end of printing log. Signed-off-by: Jun Piao --- net/9p/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/client.c b/net/9p/client.c index 5c13431..8bc8b3e 100644 --- a/net/9p/client.c +++ b/net/9p/client.c

[PATCH] 9p/net/protocol.c: return -ENOMEM when kmalloc() failed

We should return -ENOMEM to upper user when kmalloc failed to indicate accurate errno. Signed-off-by: Jun Piao --- net/9p/protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/protocol.c b/net/9p/protocol.c index 931ea00..4a1e1dd 100644 --- a/net/9p/protocol.c

Re: [V9fs-developer] [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs

Hi Dominique, This is really a *big* patch, but the modification seems no harm. And I suggest running testcases to cover this. Please see my comments below. On 2018/7/30 17:34, Dominique Martinet wrote: > From: Dominique Martinet > > 'msize' is often a power of two, or at least page-aligned,

Re: [V9fs-developer] [PATCH 2/2] net/9p: add a per-client fcall kmem_cache

Hi Dominique, Could you help paste some test result before-and-after the patch applied? And I have a little suggestion in comments below. On 2018/7/30 17:34, Dominique Martinet wrote: > From: Dominique Martinet > > Having a specific cache for the fcall allocations helps speed up > allocations

Re: [V9fs-developer] [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs

On 2018/7/31 9:12, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 31, 2018: >> This is really a *big* patch, but the modification seems no harm. And I >> suggest running testcases to cover this. Please see my comments below. > > I'm always running tests, but mor

Re: [V9fs-developer] [PATCH 2/2] net/9p: add a per-client fcall kmem_cache

On 2018/7/31 9:35, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 31, 2018: >> Could you help paste some test result before-and-after the patch applied? > > The only performance tests I did were sent to the list a couple of mails > earlier, you can find it here: > h

[PATCH v2] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

In p9_read_work(), we use spin_lock for client->lock, but misuse spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be locked in irq context, so spin_lock is enough. And that will improve the performance. Rebased on 9p-next. Signed-off-by: Jun Piao --- net/9p/client.c | 18

Re: [V9fs-developer] [PATCH v2] net/9p: Fix a deadlock case in the virtio transport

LGTM On 2018/7/17 19:03, jiangyiwen wrote: > When client has multiple threads that issue io requests > all the time, and the server has a very good performance, > it may cause cpu is running in the irq context for a long > time because it can check virtqueue has buf in the *while* > loop. > > So

Re: [PATCH v2] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

Hi Dominique, On 2018/7/17 19:56, Dominique Martinet wrote: > piaojun wrote on Tue, Jul 17, 2018: >> In p9_read_work(), we use spin_lock for client->lock, but misuse >> spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be >> locked in irq context

[PATCH] net/9p/trans_virtio.c: fix some spell mistakes in comments

Fix spelling mistake in comments of p9_virtio_zc_request(). Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index c9f2717..86077f7 100644 --- a/net/9p/trans_virtio.c +++

[PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

spin_lock is more effective for short time protection than mutex_lock, as mutex lock may cause process sleep and wake up which consume much cpu time. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git

Re: [PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

Hi Dominique, On 2018/7/18 17:54, Dominique Martinet wrote: > piaojun wrote on Wed, Jul 18, 2018: >> spin_lock is more effective for short time protection than mutex_lock, as >> mutex lock may cause process sleep and wake up which consume much cpu >> time. > > That's n

Re: [PATCH] net/9p/trans_virtio.c: replace mutex_lock with spin_lock to protect 'virtio_chan_list'

On 2018/7/19 11:36, Dominique Martinet wrote: > piaojun wrote on Thu, Jul 19, 2018: >>> piaojun wrote on Wed, Jul 18, 2018: >>> That's not a fast path operation, I don't mind changing things but I'd >>> like to understand why - these functions are only ever call

Re: [V9fs-developer] [PATCH v3] Fix a hard lockup case in the virtio transport

LGTM On 2018/7/19 15:17, jiangyiwen wrote: > When client has multiple threads that issue io requests > all the time, and the server has a very good performance, > it may cause cpu is running in the irq context for a long > time because it can check virtqueue has buf in the *while* > loop. > > So

[PATCH] scsi/virio_scsi.c: do not call virtscsi_remove_vqs() in virtscsi_init() to avoid crash bug

If some error happened before find_vqs, error branch will goto virtscsi_remove_vqs to free vqs. Actually the vqs have not been allocated successfully, so this will cause wild-pointer-free problem. So virtscsi_remove_vqs could be deleted as no error will happen after find_vqs. Signed-off-by: Jun

Re: [PATCH v3 1/2] net/9p: embed fcall in req to round down buffer allocs

LGTM On 2018/8/9 22:33, Dominique Martinet wrote: > From: Dominique Martinet > > 'msize' is often a power of two, or at least page-aligned, so avoiding > an overhead of two dozen bytes for each allocation will help the > allocator do its work and reduce memory fragmentation. > > Suggested-by:

Re: [PATCH v3 2/2] net/9p: add a per-client fcall kmem_cache

Hi Dominique, Could you help paste the test result of before-after-applied this patch in comment? And please see my comments below. On 2018/8/9 22:33, Dominique Martinet wrote: > From: Dominique Martinet > > Having a specific cache for the fcall allocations helps speed up > allocations a bit,

Re: [PATCH v3 2/2] net/9p: add a per-client fcall kmem_cache

Thanks for clearing my doubt, and you can add: Acked-by: Jun Piao On 2018/8/10 9:41, Dominique Martinet wrote: > piaojun wrote on Fri, Aug 10, 2018: >> Could you help paste the test result of before-after-applied this patch in >> comment? And please see my comments bel

[PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

chan->tag has no terminal char at last which will result in printing messy code when debugging code. So we should add '\0' for tag. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c

Re: [PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

Hi Dominique, On 2018/8/1 16:11, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 01, 2018: >> chan->tag has no terminal char at last which will result in printing messy >> code when debugging code. So we should add '\0' for tag. > > 9p is full of non null-terminate

Re: [PATCH] net/9p/trans_virtio.c: add a terminal char for mount tag

Hi Dominique and Greg, Thanks for your reviewing, and I will try to simplify other related code according your suggestions in patch v2. Thanks, Jun On 2018/8/1 20:09, Dominique Martinet wrote: > Greg Kurz wrote on Wed, Aug 01, 2018: >> So this patch basically turns chan->tag into a nul

[PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

chan->tag is Non-null terminated which will result in printing messy code when debugging code. So we should add '\0' for tag to make the code more convenient and robust. In addition, I drop char->tag_len to simplify the code. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 15

Re: [PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

Hi Dominique, On 2018/8/2 9:54, Dominique Martinet wrote: > piaojun wrote on Thu, Aug 02, 2018: >> chan->tag is Non-null terminated which will result in printing messy code >> when debugging code. So we should add '\0' for tag to make the code more >> convenient and rob

Re: [V9fs-developer] [PATCH v2] net/9p/trans_virtio.c: add null terminal for mount tag

our suggestion for the next version of the patch, or this patch is good enough? Thanks, Jun On 2018/8/2 21:23, Greg Kurz wrote: > On Thu, 2 Aug 2018 09:59:38 +0800 > piaojun wrote: > >> Hi Dominique, >> >> On 2018/8/2 9:54, Dominique Martinet wrote: >>> piaojun wrote

[PATCH v3] net/9p/trans_virtio.c: add null terminal for mount tag

chan->tag is Non-null terminated which will result in printing messy code when debugging code. So we should add '\0' for tag to make the code more convenient and robust. In addition, I drop char->tag_len to simplify the code. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 17

[PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

I found that 9pnet_virtio.ko could not be removed by rmmod command, and I could still found it by lsmod. The reason is that we forgot decrease the refcount of 9p virtio device by kobject_put. So we should put refcount in p9_virtio_remove. Signed-off-by: Jun Piao --- net/9p/trans_virtio.c | 3

Re: [PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

Hi Dominique, On 2018/8/8 16:36, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 08, 2018: >> I found that 9pnet_virtio.ko could not be removed by rmmod command, and I >> could still found it by lsmod. The reason is that we forgot decrease the >> refcount of 9p virtio

Re: [PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

Hi Dominique, On 2018/8/8 17:40, Dominique Martinet wrote: > piaojun wrote on Wed, Aug 08, 2018: >> I try to remove 9pnet_virtio.ko by 'rmmod 9pnet_virtio' as I want to >> replace it without rebooting system. > > I do that all the time when testing, it works for me. >

[PATCH] fs/9p/xattr.c: catch the error of p9_client_clunk when setting xattr failed

In my testing, v9fs_fid_xattr_set will return successfully even if the backend ext4 filesystem has no space to store xattr key-value. That will cause inconsistent behavior between front end and back end. The reason is that lsetxattr will be triggered by p9_client_clunk, and unfortunately we did

Re: [PATCH] fs/9p/xattr.c: catch the error of p9_client_clunk when setting xattr failed

Hi Dominique, On 2018/7/25 11:32, Dominique Martinet wrote: > piaojun wrote on Wed, Jul 25, 2018: >> In my testing, v9fs_fid_xattr_set will return successfully even if the >> backend ext4 filesystem has no space to store xattr key-value. That will >> cause inconsistent behav

Re: [V9fs-developer] [PATCH] fs: 9p: Adding new return type vm_fault_t

LGTM On 2018/7/2 23:49, Souptick Joarder wrote: > Use new return type vm_fault_t for page_mkwrite > handler. > > see commit 1c8f422059ae ("mm: change return type to > vm_fault_t") for reference. > > Signed-off-by: Souptick Joarder > Reviewed-by: Matthew Wilcox Acked-by: Jun Piao > --- >

[PATCH] net/9p/client.c: put refcount of trans_mod in error case in parse_opts()

>From my test, the second mount will fail after umounting successfully. The reason is that we put refcount of trans_mod in the correct case rather than the error case in parse_opts() at last. That will cause the refcount decrease to -1, and when we try to get trans_mod again in try_module_get(),

Re: [V9fs-developer] [PATCH v2 2/6] 9p: Change p9_fid_create calling convention

LGTM On 2018/7/12 5:02, Matthew Wilcox wrote: > Return NULL instead of ERR_PTR when we can't allocate a FID. The ENOSPC > return value was getting all the way back to userspace, and that's > confusing for a userspace program which isn't expecting read() to tell it > there's no space left on the

[PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

In p9_read_work(), we use spin_lock for client->lock, but misuse spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be locked in irq context, so spin_lock is enough. And that will improve the performance. Signed-off-by: Jun Piao --- net/9p/client.c | 17 +++--

Re: [V9fs-developer] [PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock

Hi Dominique, On 2018/7/12 15:01, Dominique Martinet wrote: > piaojun wrote on Thu, Jul 12, 2018: >> In p9_read_work(), we use spin_lock for client->lock, but misuse >> spin_lock_irqsave for it in p9_fid_create(). As p9_client lock won't be >> locked in irq context

[PATCH] ocfs2: free 'dummy_sc' in sc_fop_release() in case of memory leak

'sd->dbg_sock' is malloc in sc_common_open(), but not freed at the end of sc_fop_release(). Signed-off-by: Jun Piao --- fs/ocfs2/cluster/netdebug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/ocfs2/cluster/netdebug.c b/fs/ocfs2/cluster/netdebug.c index 564c504..74a21f6 100644 ---