linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>, linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [GIT PULL] Fsnotify cleanups
Date: Sat, 9 Jun 2018 10:30:23 -0700	[thread overview]
Message-ID: <CA+55aFx8YyVLaPXfpbp=omega0NsHJ1vbBBUOKEZjFkyPsBa9A@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxhKkJ=4x4UbcgZRci-qJ5XrY0NmNYfR6zyoRZH4UzQGaQ@mail.gmail.com>

On Fri, Jun 8, 2018 at 11:57 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> We embed fsnotify_obj in struct inode and fsnotify_obj_mask in struct mount.

So I'd *really* like to see just a pointer, not an embedded struct.

Yes, if you get rid of the mask from the embedded struct (so that it
only contains a pointer), you do get rid of the odd alignment issues
and the need for "packed".

But from previous experience, once you embed a structure, that
structure tends to grow. Because it can, and it's so convenient.
Suddently it has a spinlock in it too etc.

So if you can make do with just the pointer, it would actually be
nicer to expose it as such. Then you can also avoid the header file
dependency chain, because you can just pre-declare the structure (like
it does now) and

    struct fsnotify_mark_connector;
    ..
        struct fsnotify_mark_connector __rcu    *i_fsnotify_marks;

in the inode. That way the core header files don't need to worry about
the fsnotify details, and don't need to include fsnotify headers.

And we can do inode packing without having to know (not that it
happens all that often - everybody would *love* to shrink the inode
structure, but it's just hard. Because everybody also wants to put
their own data into the inode ..)

Can't the generalization code just take a pointer to a __rcu pointer
to fsnotify_mark_connector, obviating the need for the fsnotify_obj
structure definition?

             Linus

  parent reply	other threads:[~2018-06-09 17:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07 15:02 [GIT PULL] Fsnotify cleanups Jan Kara
2018-06-07 16:34 ` Linus Torvalds
2018-06-08 13:27   ` Jan Kara
2018-06-08 20:34     ` Linus Torvalds
2018-06-09  6:57       ` Amir Goldstein
2018-06-09  8:00         ` Amir Goldstein
2018-06-11 16:12           ` Jan Kara
2018-06-11 16:31             ` Amir Goldstein
2018-06-09 17:30         ` Linus Torvalds [this message]
2018-06-09 18:46           ` Amir Goldstein
2018-06-10 17:49             ` Amir Goldstein
2018-06-11 13:36               ` Jan Kara
2018-06-11 13:58                 ` Amir Goldstein
2018-06-11 16:03                   ` Jan Kara
2018-06-11 16:38                     ` Amir Goldstein
2018-06-11 19:51                       ` Amir Goldstein
2018-06-13 13:21                         ` Jan Kara
2018-06-13 13:56                           ` Amir Goldstein
2018-06-13 22:17                             ` Amir Goldstein
2018-06-22 16:44                               ` Jan Kara
2018-06-23  7:42                                 ` Amir Goldstein
2018-06-11 11:08       ` Jan Kara

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='CA+55aFx8YyVLaPXfpbp=omega0NsHJ1vbBBUOKEZjFkyPsBa9A@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).