From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: Re: [PATCH RFC v2 4/4] 9p: fix race issue in fid contention.
Date: Thu, 24 Sep 2020 11:40:09 +0300 [thread overview]
Message-ID: <20200924084009.GR18329@kadam> (raw)
In-Reply-To: <CY4PR1101MB2326913B505A1FA1378FFFB9E7390@CY4PR1101MB2326.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]
On Thu, Sep 24, 2020 at 08:29:48AM +0000, Reshetova, Elena wrote:
> > > >> net/9p/client.c:1469:6-25: atomic_dec_and_test variation before object free at
> > line 1497.
> >
> >
> > This warning was confusing for me. Perhaps a better wording would be:
> > "Use refcount_t instead of atomic_t for refcounting"?
>
> The reason we didn't want to have such a strong statement, because this detection
> can give false positives, i.e. suggest converting smth that should be left as atomic_t.
> So, it should be always carefully analyzed by a code author before just plainly converting.
> Also, this way it gives you the background on which exact pattern (we have a number
> of them) triggered this case, and if we see that one pattern gives too much mistakes,
> we can remove/update it. But atomic_dec_and_test usage before freeing the object is
> actually one of the most common cases., so maybe I can add a statement saying
> "consider using refcount_t instead of atomic_t"?
That works. The original message was just confusing and I spent some
time trying to spot a bug in the code. And really the code is fine
(probably. maybe. no reason to think it's not fine). It's just that
refcounting is preferred as a hardening measure.
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild-all@lists.01.org
Subject: Re: [kbuild] Re: [PATCH RFC v2 4/4] 9p: fix race issue in fid contention.
Date: Thu, 24 Sep 2020 11:40:09 +0300 [thread overview]
Message-ID: <20200924084009.GR18329@kadam> (raw)
In-Reply-To: <CY4PR1101MB2326913B505A1FA1378FFFB9E7390@CY4PR1101MB2326.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]
On Thu, Sep 24, 2020 at 08:29:48AM +0000, Reshetova, Elena wrote:
> > > >> net/9p/client.c:1469:6-25: atomic_dec_and_test variation before object free at
> > line 1497.
> >
> >
> > This warning was confusing for me. Perhaps a better wording would be:
> > "Use refcount_t instead of atomic_t for refcounting"?
>
> The reason we didn't want to have such a strong statement, because this detection
> can give false positives, i.e. suggest converting smth that should be left as atomic_t.
> So, it should be always carefully analyzed by a code author before just plainly converting.
> Also, this way it gives you the background on which exact pattern (we have a number
> of them) triggered this case, and if we see that one pattern gives too much mistakes,
> we can remove/update it. But atomic_dec_and_test usage before freeing the object is
> actually one of the most common cases., so maybe I can add a statement saying
> "consider using refcount_t instead of atomic_t"?
That works. The original message was just confusing and I spent some
time trying to spot a bug in the code. And really the code is fine
(probably. maybe. no reason to think it's not fine). It's just that
refcounting is preferred as a hardening measure.
regards,
dan carpenter
next prev parent reply other threads:[~2020-09-24 8:40 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-24 4:50 [PATCH RFC v2 4/4] 9p: fix race issue in fid contention kernel test robot
2020-09-24 7:01 ` Dan Carpenter
2020-09-24 7:01 ` [kbuild] " Dan Carpenter
2020-09-24 8:29 ` Reshetova, Elena
2020-09-24 8:40 ` Dan Carpenter [this message]
2020-09-24 8:40 ` Dan Carpenter
2020-09-24 10:29 ` Reshetova, Elena
-- strict thread matches above, loose matches on Subject: below --
2020-09-23 14:11 [PATCH RFC v2 0/4] 9p: fix open-unlink-f*syscall bug Jianyong Wu
2020-09-23 14:11 ` [PATCH RFC v2 4/4] 9p: fix race issue in fid contention Jianyong Wu
2020-09-23 14:49 ` Dominique Martinet
2020-09-24 8:38 ` Jianyong Wu
2020-09-24 8:56 ` Greg Kurz
2020-09-24 9:51 ` Dominique Martinet
2020-09-25 9:49 ` Jianyong Wu
2020-11-03 10:41 ` Dominique Martinet
2020-11-04 11:32 ` Christian Schoenebeck
2020-11-04 11:57 ` Dominique Martinet
2020-11-05 12:32 ` Christian Schoenebeck
2020-11-05 7:05 ` Jianyong Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200924084009.GR18329@kadam \
--to=dan.carpenter@oracle.com \
--cc=kbuild@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.