Re: [PATCH v1 00/10] vfs: getname/putname overhaul
by Jeff Layton
On Sat, 8 Sep 2012 05:08:50 +0200
Andi Kleen <andi(a)firstfloor.org> wrote:
> On Fri, Sep 07, 2012 at 08:54:18PM -0400, Jeff Layton wrote:
> > On Fri, 07 Sep 2012 14:26:56 -0700
> > Andi Kleen <andi(a)firstfloor.org> wrote:
> >
> > > Jeff Layton <jlayton(a)redhat.com> writes:
> > >
> > > > This patchset is a first pass at overhauling the getname/putname
> > > > interface to use a struct. The idea here is to add a new getname_info
> > > > struct that allow us to pass around some auxillary info along with
> > > > the string that getname() returns.
> > >
> > > Couldn't you just use some of the free pointers in struct page?
> > > (lru etc.)
> > >
> > > -Andi
> > >
> >
> > We could do that if these were page allocations. They're not, however.
> > __getname() does a PATH_MAX size allocation out of a slabcache. I get
>
> Ok I suppose slab is faster. In this case it's better to track
> separately I agree.
>
Ummm...stupid question...
I could see that allocating out of the slab would mean less waste when
you have >4k pages, but why would it be faster than just allocating a
page directly?
Also, by "track separately" do you mean that you think I should drop
patch 9 in this series and just do two allocations for a getname in all
cases?
--
Jeff Layton <jlayton(a)redhat.com>
13 years, 2 months
- 1
- 0
Re: [PATCH v1 00/10] vfs: getname/putname overhaul
by Jeff Layton
On Fri, 07 Sep 2012 14:26:56 -0700
Andi Kleen <andi(a)firstfloor.org> wrote:
> Jeff Layton <jlayton(a)redhat.com> writes:
>
> > This patchset is a first pass at overhauling the getname/putname
> > interface to use a struct. The idea here is to add a new getname_info
> > struct that allow us to pass around some auxillary info along with
> > the string that getname() returns.
>
> Couldn't you just use some of the free pointers in struct page?
> (lru etc.)
>
> -Andi
>
We could do that if these were page allocations. They're not, however.
__getname() does a PATH_MAX size allocation out of a slabcache. I get
the impression that at one point, this was done with page allocations.
I assume that this was changed over to a slabcache since not all arches
have a 4k PAGE_SIZE.
That's not a bad idea though. We could switch __getname over to doing a
page allocation instead, but it would mean quite a bit of wasted space
on arches with >4k pages. That said, these allocations are generally
quite short-lived so maybe that's not such a big deal.
--
Jeff Layton <jlayton(a)redhat.com>
13 years, 2 months
- 1
- 0
[PATCH v1 00/10] vfs: getname/putname overhaul
by Jeff Layton
This patchset is a first pass at overhauling the getname/putname
interface to use a struct. The idea here is to add a new getname_info
struct that allow us to pass around some auxillary info along with
the string that getname() returns.
This allows us to do some interesting things:
- no need to walk the list of audit_names in certain cases since we
can store a pointer to the correct audit_name
- we can now call getname() more than once on a userland string. Since
we track the original userland pointer, we can avoid doing a second
allocation, and can instead fill out the getname_info from the
audit_names struct. That makes the ESTALE patchset cleaner, and doesn't
explode out the list of getname() callers like the last set.
- eventually we might be able to track the length of the parent portion
of the string so the audit code doesn't need to walk it again. I
haven't implemented that yet, but it doesn't look too hard to do.
This is based on top of Al's signal.git#execve2 branch, with my most
recent audit series on top of that. Al is working on unifying much of
the execve code, which will reduce the number of getname callers greatly.
This set is still preliminary since Al's set isn't complete yet and will
probably need to be respun again once he's completed that work. That
should shrink patch #4 since we'll have fewer getname callers to deal
with at that point.
This set is based on top of my audit overhaul patchset (posted earlier
today). I'll also be posting a respun version of my ESTALE retry
patchset soon that's based on top of this one.
While this all seems to work correctly, I have my doubts about patch #9
in this series. That was suggested by Al and should make it so that we
only need a single allocation per getname() call in most cases. OTOH, it
adds a rarely traveled codepath that could be a source of bugs in the
future.
Jeff Layton (10):
vfs: allocate page instead of names_cache buffer in mount_block_root
vfs: make dir_name arg to do_mount a const char *
acct: constify the name arg to acct_on
vfs: define getname_info struct and have getname() return it
audit: allow audit code to satisfy getname requests from its
names_list
vfs: turn do_path_lookup into wrapper around getname_info variant
vfs: make path_openat take a getname_info pointer
audit: make audit_inode take getname_info
vfs: embed getname_info inside of names_cache allocation if possible
vfs: unexport getname and putname symbols
arch/alpha/kernel/osf_sys.c | 16 +--
arch/avr32/kernel/process.c | 4 +-
arch/blackfin/kernel/process.c | 4 +-
arch/c6x/kernel/process.c | 4 +-
arch/cris/arch-v10/kernel/process.c | 4 +-
arch/cris/arch-v32/kernel/process.c | 4 +-
arch/frv/kernel/process.c | 4 +-
arch/h8300/kernel/process.c | 4 +-
arch/hexagon/kernel/syscall.c | 4 +-
arch/ia64/kernel/process.c | 4 +-
arch/m32r/kernel/process.c | 4 +-
arch/m68k/kernel/process.c | 4 +-
arch/microblaze/kernel/sys_microblaze.c | 4 +-
arch/mips/kernel/linux32.c | 4 +-
arch/mips/kernel/syscall.c | 4 +-
arch/mn10300/kernel/process.c | 4 +-
arch/openrisc/kernel/process.c | 4 +-
arch/parisc/hpux/fs.c | 4 +-
arch/parisc/kernel/process.c | 4 +-
arch/parisc/kernel/sys_parisc32.c | 4 +-
arch/score/kernel/sys_score.c | 4 +-
arch/sh/kernel/process_32.c | 4 +-
arch/sh/kernel/process_64.c | 4 +-
arch/sparc/kernel/process_32.c | 4 +-
arch/sparc/kernel/process_64.c | 4 +-
arch/sparc/kernel/sys_sparc32.c | 4 +-
arch/tile/kernel/process.c | 8 +-
arch/unicore32/kernel/sys.c | 4 +-
arch/xtensa/kernel/process.c | 4 +-
fs/compat.c | 12 +-
fs/exec.c | 13 +-
fs/filesystems.c | 4 +-
fs/internal.h | 4 +-
fs/namei.c | 214 +++++++++++++++++++++-----------
fs/namespace.c | 6 +-
fs/open.c | 33 ++++-
fs/quota/quota.c | 4 +-
include/linux/audit.h | 26 ++--
include/linux/fs.h | 23 +++-
init/do_mounts.c | 7 +-
ipc/mqueue.c | 9 +-
kernel/acct.c | 6 +-
kernel/auditsc.c | 124 +++++++++++-------
mm/swapfile.c | 11 +-
44 files changed, 392 insertions(+), 236 deletions(-)
--
1.7.11.4
13 years, 2 months
- 1
- 10
[PATCH v6 00/10] audit: overhaul audit_names handling to allow for retrying on path-based syscalls
by Jeff Layton
This patchset is another minor respin of the series that I posted June
26th.
The main impetus of this series to to allow the audit_names
infrastructure to properly handle the situation where the lookup and
operation on a path-based syscall is retried.
Along with that, there are a number of bugfixes in this patch, as well
as a few minor optimizations.
This series is available via the "audit" branch of my git tree as well:
git://git.samba.org/jlayton/linux.git audit
I'd like to see this patchset go into 3.7 if at all possible.
Eric Paris (1):
audit: make audit_compare_dname_path use parent_len helper
Jeff Layton (9):
audit: remove unnecessary NULL ptr checks from do_path_lookup
audit: pass in dentry to audit_copy_inode wherever possible
audit: no need to walk list in audit_inode if name is NULL
audit: reverse arguments to audit_inode_child
audit: add a new "type" field to audit_names struct
audit: set the name_len in audit_inode for parent lookups
audit: remove dirlen argument to audit_compare_dname_path
audit: optimize audit_compare_dname_path
audit: overhaul __audit_inode_child to accomodate retrying
fs/btrfs/ioctl.c | 2 +-
fs/namei.c | 22 ++++-----
fs/open.c | 4 +-
fs/xattr.c | 8 ++--
include/linux/audit.h | 36 +++++++++-----
include/linux/fsnotify.h | 8 ++--
ipc/mqueue.c | 8 ++--
kernel/audit.h | 7 ++-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 65 +++++++++++++++++---------
kernel/auditsc.c | 119 +++++++++++++++++++++++++++++------------------
11 files changed, 170 insertions(+), 112 deletions(-)
--
1.7.11.4
13 years, 2 months
- 1
- 10