All of lore.kernel.org
 help / color / mirror / Atom feed
From: "'tj@kernel.org'" <tj@kernel.org>
To: "Hatayama, Daisuke" <d.hatayama@jp.fujitsu.com>
Cc: "'gregkh@linuxfoundation.org'" <gregkh@linuxfoundation.org>,
	"Okajima, Toshiyuki" <toshi.okajima@jp.fujitsu.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"'ebiederm@aristanetworks.com'" <ebiederm@aristanetworks.com>
Subject: Re: [RESEND PATCH v2] kernfs: fix dentry unexpected skip
Date: Tue, 29 May 2018 09:26:27 -0700	[thread overview]
Message-ID: <20180529162627.GH1351649@devbig577.frc2.facebook.com> (raw)
In-Reply-To: <33710E6CAA200E4583255F4FB666C4E21B63D491@G01JPEXMBYT03>

Hello,

On Mon, May 28, 2018 at 12:54:03PM +0000, Hatayama, Daisuke wrote:
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 89d1dc1..3aeeb7a 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1621,8 +1621,10 @@ static int kernfs_dir_fop_release(struct inode *inode, struct file *filp)
>  static struct kernfs_node *kernfs_dir_next_pos(const void *ns,
>  	struct kernfs_node *parent, ino_t ino, struct kernfs_node *pos)
>  {
> +	struct kernfs_node *orig = pos;
> +
>  	pos = kernfs_dir_pos(ns, parent, ino, pos);
> -	if (pos) {
> +	if (pos && kernfs_sd_compare(pos, orig) <= 0) {

Hmm... the code seems a bit unintuitive to me and I wonder whether
it's because there are two identical skipping loops in
kernfs_dir_pos() and kernfs_dir_next_pos() and we're now trying to
selectively disable one of them.  Wouldn't it make more sense to get
rid of it from kernfs_dir_pos() and skip explicitly only when
necessary?

Thanks.

-- 
tejun

  parent reply	other threads:[~2018-05-29 16:26 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-28 12:54 [RESEND PATCH v2] kernfs: fix dentry unexpected skip Hatayama, Daisuke
2018-05-28 13:08 ` Hatayama, Daisuke
2018-05-29 16:26 ` 'tj@kernel.org' [this message]
2018-06-01  9:25   ` Hatayama, Daisuke
2018-06-01 17:07     ` 'tj@kernel.org'
2018-06-04  9:46       ` Hatayama, Daisuke
2018-06-02 17:25 ` Eric W. Biederman
2018-06-03 18:51   ` [CFT][PATCH] kernfs: Correct kernfs directory seeks Eric W. Biederman
2018-06-04  9:34     ` Hatayama, Daisuke
2018-06-04 14:44       ` Eric W. Biederman
2018-06-05  2:02         ` Eric W. Biederman
2018-06-05  5:52           ` Hatayama, Daisuke
2018-06-05  5:45         ` Hatayama, Daisuke
2018-06-05 15:31           ` Eric W. Biederman
2018-06-05 15:42             ` 'tj@kernel.org'
2018-06-05 17:47               ` Eric W. Biederman
2018-06-07 18:36                 ` Al Viro

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=20180529162627.GH1351649@devbig577.frc2.facebook.com \
    --to=tj@kernel.org \
    --cc=d.hatayama@jp.fujitsu.com \
    --cc=ebiederm@aristanetworks.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=toshi.okajima@jp.fujitsu.com \
    /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.