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, so spin_lock is enough. And that will improve the
>> performance.
>
> Agreed on principle, see remark below
>
>> Signed-off-by: Jun Piao
>> ---
>> net/9p/client.c | 17 +++--
>> net/9p/trans_fd.c | 7 +++
>> 2 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/9p/client.c b/net/9p/client.c
>> index 8bc8b3e..b05cbfc 100644
>> --- a/net/9p/client.c
>> +++ b/net/9p/client.c
>> @@ -260,7 +260,6 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
>> static struct p9_req_t *
>> p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
>> {
>> -unsigned long flags;
>> int row, col;
>> struct p9_req_t *req;
>> int alloc_msize = min(c->msize, max_size);
>> @@ -270,7 +269,7 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
>> tag++;
>>
>> if (tag >= c->max_tag) {
>> -spin_lock_irqsave(>lock, flags);
>> +spin_lock(>lock);
>
> This code doesn't exist anymore with Matthew's idr rework, could you
> submit that patch based on top of my 9p-next branch?
> (unless you really want Andrew to take this for the next 4.18-rc, but
> I'm not convinced this qualifies)
OK, I will rebase my patch and resend later.
Thanks,
Jun
>
> Please see my "Current 9P patches - test branch" for details:
> https://sourceforge.net/p/v9fs/mailman/message/36365359/
>
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, so spin_lock is enough. And that will improve the
>> performance.
>
> Agreed on principle, see remark below
>
>> Signed-off-by: Jun Piao
>> ---
>> net/9p/client.c | 17 +++--
>> net/9p/trans_fd.c | 7 +++
>> 2 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/9p/client.c b/net/9p/client.c
>> index 8bc8b3e..b05cbfc 100644
>> --- a/net/9p/client.c
>> +++ b/net/9p/client.c
>> @@ -260,7 +260,6 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
>> static struct p9_req_t *
>> p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
>> {
>> -unsigned long flags;
>> int row, col;
>> struct p9_req_t *req;
>> int alloc_msize = min(c->msize, max_size);
>> @@ -270,7 +269,7 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
>> tag++;
>>
>> if (tag >= c->max_tag) {
>> -spin_lock_irqsave(>lock, flags);
>> +spin_lock(>lock);
>
> This code doesn't exist anymore with Matthew's idr rework, could you
> submit that patch based on top of my 9p-next branch?
> (unless you really want Andrew to take this for the next 4.18-rc, but
> I'm not convinced this qualifies)
OK, I will rebase my patch and resend later.
Thanks,
Jun
>
> Please see my "Current 9P patches - test branch" for details:
> https://sourceforge.net/p/v9fs/mailman/message/36365359/
>
Re: [V9fs-developer] [PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock
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, so spin_lock is enough. And that will improve the
> performance.
Agreed on principle, see remark below
> Signed-off-by: Jun Piao
> ---
> net/9p/client.c | 17 +++--
> net/9p/trans_fd.c | 7 +++
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 8bc8b3e..b05cbfc 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -260,7 +260,6 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
> static struct p9_req_t *
> p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
> {
> - unsigned long flags;
> int row, col;
> struct p9_req_t *req;
> int alloc_msize = min(c->msize, max_size);
> @@ -270,7 +269,7 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
> tag++;
>
> if (tag >= c->max_tag) {
> - spin_lock_irqsave(>lock, flags);
> + spin_lock(>lock);
This code doesn't exist anymore with Matthew's idr rework, could you
submit that patch based on top of my 9p-next branch?
(unless you really want Andrew to take this for the next 4.18-rc, but
I'm not convinced this qualifies)
Please see my "Current 9P patches - test branch" for details:
https://sourceforge.net/p/v9fs/mailman/message/36365359/
--
Dominique Martinet
Re: [V9fs-developer] [PATCH] net/9p/client.c: fix misuse of spin_lock_irqsave for p9_client lock
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, so spin_lock is enough. And that will improve the
> performance.
Agreed on principle, see remark below
> Signed-off-by: Jun Piao
> ---
> net/9p/client.c | 17 +++--
> net/9p/trans_fd.c | 7 +++
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 8bc8b3e..b05cbfc 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -260,7 +260,6 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
> static struct p9_req_t *
> p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
> {
> - unsigned long flags;
> int row, col;
> struct p9_req_t *req;
> int alloc_msize = min(c->msize, max_size);
> @@ -270,7 +269,7 @@ static struct p9_fcall *p9_fcall_alloc(int alloc_msize)
> tag++;
>
> if (tag >= c->max_tag) {
> - spin_lock_irqsave(>lock, flags);
> + spin_lock(>lock);
This code doesn't exist anymore with Matthew's idr rework, could you
submit that patch based on top of my 9p-next branch?
(unless you really want Andrew to take this for the next 4.18-rc, but
I'm not convinced this qualifies)
Please see my "Current 9P patches - test branch" for details:
https://sourceforge.net/p/v9fs/mailman/message/36365359/
--
Dominique Martinet
