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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180712070122.GB7639@nautica>
Date:   Thu, 12 Jul 2018 09:01:22 +0200
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     piaojun <piaojun@...wei.com>
Cc:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        ericvh@...il.com, rminnich@...dia.gov, lucho@...kov.net,
        v9fs-developer@...ts.sourceforge.net,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: 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 <piaojun@...wei.com>
> ---
>  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(&c->lock, flags);
> +		spin_lock(&c->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