Skip to content

Commit

Permalink
fanotify: Store fanotify handles differently
Browse files Browse the repository at this point in the history 
Currently, struct fanotify_fid groups fsid and file handle and is
unioned together with struct path to save space. Also there is fh_type
and fh_len directly in struct fanotify_event to avoid padding overhead.
In the follwing patches, we will be adding more event types and this
packing makes code difficult to follow. So unpack everything and create
struct fanotify_fh which groups members logically related to file handle
to make code easier to follow. In the following patch we will pack
things again differently to make events smaller.

Signed-off-by: Jan Kara <jack@suse.cz>
  • Loading branch information
@jankara
jankara committed Mar 25, 2020
1 parent a741c2f commit afc894c
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 106 deletions.
97 changes: 63 additions & 34 deletions fs/notify/fanotify/fanotify.c
View file
Expand Up @@ -17,6 +17,31 @@

#include "fanotify.h"

static bool fanotify_path_equal(struct path *p1, struct path *p2)
{
return p1->mnt == p2->mnt && p1->dentry == p2->dentry;
}

static inline bool fanotify_fsid_equal(__kernel_fsid_t *fsid1,
__kernel_fsid_t *fsid2)
{
return fsid1->val[0] == fsid1->val[0] && fsid2->val[1] == fsid2->val[1];
}

static bool fanotify_fh_equal(struct fanotify_fh *fh1,
struct fanotify_fh *fh2)
{
if (fh1->type != fh2->type || fh1->len != fh2->len)
return false;

/* Do not merge events if we failed to encode fh */
if (fh1->type == FILEID_INVALID)
return false;

return !fh1->len ||
!memcmp(fanotify_fh_buf(fh1), fanotify_fh_buf(fh2), fh1->len);
}

static bool should_merge(struct fsnotify_event *old_fsn,
struct fsnotify_event *new_fsn)
{
Expand All @@ -27,12 +52,12 @@ static bool should_merge(struct fsnotify_event *old_fsn,
new = FANOTIFY_E(new_fsn);

if (old_fsn->objectid != new_fsn->objectid || old->pid != new->pid ||
old->fh_type != new->fh_type || old->fh_len != new->fh_len)
old->fh.type != new->fh.type)
return false;

if (fanotify_event_has_path(old)) {
return old->path.mnt == new->path.mnt &&
old->path.dentry == new->path.dentry;
return fanotify_path_equal(fanotify_event_path(old),
fanotify_event_path(new));
} else if (fanotify_event_has_fid(old)) {
/*
* We want to merge many dirent events in the same dir (i.e.
Expand All @@ -42,8 +67,11 @@ static bool should_merge(struct fsnotify_event *old_fsn,
* mask FAN_CREATE|FAN_DELETE|FAN_ONDIR if it describes mkdir+
* unlink pair or rmdir+create pair of events.
*/
return (old->mask & FS_ISDIR) == (new->mask & FS_ISDIR) &&
fanotify_fid_equal(&old->fid, &new->fid, old->fh_len);
if ((old->mask & FS_ISDIR) != (new->mask & FS_ISDIR))
return false;

return fanotify_fsid_equal(&old->fsid, &new->fsid) &&
fanotify_fh_equal(&old->fh, &new->fh);
}

/* Do not merge events if we failed to encode fid */
Expand Down Expand Up @@ -213,15 +241,14 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
return test_mask & user_mask;
}

static int fanotify_encode_fid(struct fanotify_event *event,
struct inode *inode, gfp_t gfp,
__kernel_fsid_t *fsid)
static void fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
gfp_t gfp)
{
struct fanotify_fid *fid = &event->fid;
int dwords, bytes = 0;
int err, type;
int dwords, type, bytes = 0;
char *ext_buf = NULL;
void *buf = fh->buf;
int err;

fid->ext_fh = NULL;
dwords = 0;
err = -ENOENT;
type = exportfs_encode_inode_fh(inode, NULL, &dwords, NULL);
Expand All @@ -232,31 +259,32 @@ static int fanotify_encode_fid(struct fanotify_event *event,
if (bytes > FANOTIFY_INLINE_FH_LEN) {
/* Treat failure to allocate fh as failure to allocate event */
err = -ENOMEM;
fid->ext_fh = kmalloc(bytes, gfp);
if (!fid->ext_fh)
ext_buf = kmalloc(bytes, gfp);
if (!ext_buf)
goto out_err;

*fanotify_fh_ext_buf_ptr(fh) = ext_buf;
buf = ext_buf;
}

type = exportfs_encode_inode_fh(inode, fanotify_fid_fh(fid, bytes),
&dwords, NULL);
type = exportfs_encode_inode_fh(inode, buf, &dwords, NULL);
err = -EINVAL;
if (!type || type == FILEID_INVALID || bytes != dwords << 2)
goto out_err;

fid->fsid = *fsid;
event->fh_len = bytes;
fh->type = type;
fh->len = bytes;

return type;
return;

out_err:
pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
"type=%d, bytes=%d, err=%i)\n",
fsid->val[0], fsid->val[1], type, bytes, err);
kfree(fid->ext_fh);
fid->ext_fh = NULL;
event->fh_len = 0;

return FILEID_INVALID;
pr_warn_ratelimited("fanotify: failed to encode fid (type=%d, len=%d, err=%i)\n",
type, bytes, err);
kfree(ext_buf);
*fanotify_fh_ext_buf_ptr(fh) = NULL;
/* Report the event without a file identifier on encode error */
fh->type = FILEID_INVALID;
fh->len = 0;
}

/*
Expand Down Expand Up @@ -326,16 +354,17 @@ init: __maybe_unused
event->pid = get_pid(task_pid(current));
else
event->pid = get_pid(task_tgid(current));
event->fh_len = 0;
event->fh.len = 0;
if (id && FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
/* Report the event without a file identifier on encode error */
event->fh_type = fanotify_encode_fid(event, id, gfp, fsid);
event->fsid = *fsid;
if (id)
fanotify_encode_fh(&event->fh, id, gfp);
} else if (path) {
event->fh_type = FILEID_ROOT;
event->fh.type = FILEID_ROOT;
event->path = *path;
path_get(path);
} else {
event->fh_type = FILEID_INVALID;
event->fh.type = FILEID_INVALID;
event->path.mnt = NULL;
event->path.dentry = NULL;
}
Expand Down Expand Up @@ -483,8 +512,8 @@ static void fanotify_free_event(struct fsnotify_event *fsn_event)
event = FANOTIFY_E(fsn_event);
if (fanotify_event_has_path(event))
path_put(&event->path);
else if (fanotify_event_has_ext_fh(event))
kfree(event->fid.ext_fh);
else if (fanotify_fh_has_ext_buf(&event->fh))
kfree(fanotify_fh_ext_buf(&event->fh));
put_pid(event->pid);
if (fanotify_is_perm_event(event->mask)) {
kmem_cache_free(fanotify_perm_event_cachep,
Expand Down
115 changes: 62 additions & 53 deletions fs/notify/fanotify/fanotify.h
View file
Expand Up @@ -18,39 +18,37 @@ enum {

/*
* 3 dwords are sufficient for most local fs (64bit ino, 32bit generation).
* For 32bit arch, fid increases the size of fanotify_event by 12 bytes and
* fh_* fields increase the size of fanotify_event by another 4 bytes.
* For 64bit arch, fid increases the size of fanotify_fid by 8 bytes and
* fh_* fields are packed in a hole after mask.
* fh buf should be dword aligned. On 64bit arch, the ext_buf pointer is
* stored in either the first or last 2 dwords.
*/
#if BITS_PER_LONG == 32
#define FANOTIFY_INLINE_FH_LEN (3 << 2)
#else
#define FANOTIFY_INLINE_FH_LEN (4 << 2)
#endif

struct fanotify_fid {
__kernel_fsid_t fsid;
union {
unsigned char fh[FANOTIFY_INLINE_FH_LEN];
unsigned char *ext_fh;
};
};
struct fanotify_fh {
unsigned char buf[FANOTIFY_INLINE_FH_LEN];
u8 type;
u8 len;
} __aligned(4);

static inline bool fanotify_fh_has_ext_buf(struct fanotify_fh *fh)
{
return fh->len > FANOTIFY_INLINE_FH_LEN;
}

static inline void *fanotify_fid_fh(struct fanotify_fid *fid,
unsigned int fh_len)
static inline char **fanotify_fh_ext_buf_ptr(struct fanotify_fh *fh)
{
return fh_len <= FANOTIFY_INLINE_FH_LEN ? fid->fh : fid->ext_fh;
BUILD_BUG_ON(__alignof__(char *) - 4 + sizeof(char *) >
FANOTIFY_INLINE_FH_LEN);
return (char **)ALIGN((unsigned long)(fh->buf), __alignof__(char *));
}

static inline bool fanotify_fid_equal(struct fanotify_fid *fid1,
struct fanotify_fid *fid2,
unsigned int fh_len)
static inline void *fanotify_fh_ext_buf(struct fanotify_fh *fh)
{
return fid1->fsid.val[0] == fid2->fsid.val[0] &&
fid1->fsid.val[1] == fid2->fsid.val[1] &&
!memcmp(fanotify_fid_fh(fid1, fh_len),
fanotify_fid_fh(fid2, fh_len), fh_len);
return *fanotify_fh_ext_buf_ptr(fh);
}

static inline void *fanotify_fh_buf(struct fanotify_fh *fh)
{
return fanotify_fh_has_ext_buf(fh) ? fanotify_fh_ext_buf(fh) : fh->buf;
}

/*
Expand All @@ -62,50 +60,53 @@ struct fanotify_event {
struct fsnotify_event fse;
u32 mask;
/*
* Those fields are outside fanotify_fid to pack fanotify_event nicely
* on 64bit arch and to use fh_type as an indication of whether path
* or fid are used in the union:
* FILEID_ROOT (0) for path, > 0 for fid, FILEID_INVALID for neither.
* With FAN_REPORT_FID, we do not hold any reference on the
* victim object. Instead we store its NFS file handle and its
* filesystem's fsid as a unique identifier.
*/
__kernel_fsid_t fsid;
struct fanotify_fh fh;
/*
* We hold ref to this path so it may be dereferenced at any
* point during this object's lifetime
*/
u8 fh_type;
u8 fh_len;
u16 pad;
union {
/*
* We hold ref to this path so it may be dereferenced at any
* point during this object's lifetime
*/
struct path path;
/*
* With FAN_REPORT_FID, we do not hold any reference on the
* victim object. Instead we store its NFS file handle and its
* filesystem's fsid as a unique identifier.
*/
struct fanotify_fid fid;
};
struct path path;
struct pid *pid;
};

static inline bool fanotify_event_has_path(struct fanotify_event *event)
{
return event->fh_type == FILEID_ROOT;
return event->fh.type == FILEID_ROOT;
}

static inline bool fanotify_event_has_fid(struct fanotify_event *event)
{
return event->fh_type != FILEID_ROOT &&
event->fh_type != FILEID_INVALID;
return event->fh.type != FILEID_ROOT &&
event->fh.type != FILEID_INVALID;
}

static inline bool fanotify_event_has_ext_fh(struct fanotify_event *event)
static inline __kernel_fsid_t *fanotify_event_fsid(struct fanotify_event *event)
{
return fanotify_event_has_fid(event) &&
event->fh_len > FANOTIFY_INLINE_FH_LEN;
if (fanotify_event_has_fid(event))
return &event->fsid;
else
return NULL;
}

static inline void *fanotify_event_fh(struct fanotify_event *event)
static inline struct fanotify_fh *fanotify_event_object_fh(
struct fanotify_event *event)
{
return fanotify_fid_fh(&event->fid, event->fh_len);
if (fanotify_event_has_fid(event))
return &event->fh;
else
return NULL;
}

static inline int fanotify_event_object_fh_len(struct fanotify_event *event)
{
struct fanotify_fh *fh = fanotify_event_object_fh(event);

return fh ? fh->len : 0;
}

/*
Expand Down Expand Up @@ -139,6 +140,14 @@ static inline struct fanotify_event *FANOTIFY_E(struct fsnotify_event *fse)
return container_of(fse, struct fanotify_event, fse);
}

static inline struct path *fanotify_event_path(struct fanotify_event *event)
{
if (fanotify_event_has_path(event))
return &event->path;
else
return NULL;
}

struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
struct inode *inode, u32 mask,
const void *data, int data_type,
Expand Down

0 comments on commit afc894c

Please sign in to comment.