Message ID | 20200702125744.10535-5-amir73il@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fanotify events with name info | expand |
On Thu 02-07-20 15:57:38, Amir Goldstein wrote: > Similar to events "on child" to watching directory, send event "on child" > with parent/name info if sb/mount/non-dir marks are interested in > parent/name info. > > The FS_EVENT_ON_CHILD flag can be set on sb/mount/non-dir marks to specify > interest in parent/name info for events on non-directory inodes. > > Events on "orphan" children (disconnected dentries) are sent without > parent/name info. > > Events on direcories are send with parent/name info only if the parent > directory is watching. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/notify/fsnotify.c | 50 +++++++++++++++++++++++--------- > include/linux/fsnotify.h | 10 +++++-- > include/linux/fsnotify_backend.h | 32 +++++++++++++++++--- > 3 files changed, 73 insertions(+), 19 deletions(-) > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 7c6e624b24c9..6683c77a5b13 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -144,27 +144,55 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode) > > /* > * Notify this dentry's parent about a child's events with child name info > - * if parent is watching. > - * Notify only the child without name info if parent is not watching. > + * if parent is watching or if inode/sb/mount are interested in events with > + * parent and name info. > + * > + * Notify only the child without name info if parent is not watching and > + * inode/sb/mount are not interested in events with parent and name info. > */ > int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, > int data_type) > { Ugh, I have to say this function is now pretty difficult to digest. I was staring at it for an hour before I could make some sence of it... > + const struct path *path = fsnotify_data_path(data, data_type); > + struct mount *mnt = path ? real_mount(path->mnt) : NULL; > struct inode *inode = d_inode(dentry); > struct dentry *parent; > + bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; > + __u32 p_mask, test_mask, marks_mask = 0; > struct inode *p_inode; > int ret = 0; > > + /* > + * Do inode/sb/mount care about parent and name info on non-dir? > + * Do they care about any event at all? > + */ > + if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && > + (!mnt || !mnt->mnt_fsnotify_marks)) { > + if (!parent_watched) > + return 0; > + } else if (!(mask & FS_ISDIR) && !IS_ROOT(dentry)) { > + marks_mask |= fsnotify_want_parent(inode->i_fsnotify_mask); > + marks_mask |= fsnotify_want_parent(inode->i_sb->s_fsnotify_mask); > + if (mnt) > + marks_mask |= fsnotify_want_parent(mnt->mnt_fsnotify_mask); > + } OK, so AFAIU at this point (mask & marks_mask) tells us whether we need to grab parent because some mark needs parent into reported. Correct? Maybe I'd rename fsnotify_want_parent() (which seems like it returns bool) to fsnotify_parent_needed_mask() or something like that. Also I'd hide all those checks in a helper function like: fsnotify_event_needs_parent(mnt, inode, mask) So we'd then have just something like: if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && (!mnt || !mnt->mnt_fsnotify_marks) && !parent_watched) return 0; if (!parent_watched && !fsnotify_event_needs_parent(mnt, inode, mask)) goto notify_child; ... > + > parent = NULL; > - if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) > + test_mask = mask & FS_EVENTS_POSS_TO_PARENT; > + if (!(marks_mask & test_mask) && !parent_watched) > goto notify_child; > > + /* Does parent inode care about events on children? */ > parent = dget_parent(dentry); > p_inode = parent->d_inode; > + p_mask = fsnotify_inode_watches_children(p_inode); > > - if (unlikely(!fsnotify_inode_watches_children(p_inode))) { > + if (p_mask) > + marks_mask |= p_mask; > + else if (unlikely(parent_watched)) > __fsnotify_update_child_dentry_flags(p_inode); > - } else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) { > + > + if ((marks_mask & test_mask) && p_inode != inode) { ^^ this is effectively !IS_ROOT(dentry), isn't it? But since you've checked that above can it ever be that p_inode == inode? Also if marks_mask & test_mask == 0, why do you go to a branch that notifies child? There shouldn't be anything to report there either. Am I missing something? ... Oh, I see, the FS_EVENTS_POSS_TO_PARENT masking above could cause that child is still interested. Because mask & marks_mask can still contain something. Maybe it would be more comprehensible if we restructure the above like: p_mask = fsnotify_inode_watches_children(p_inode); if (unlikely(parent_watched && !p_mask)) __fsnotify_update_child_dentry_flags(p_inode); /* * Include parent in notification either if some notification * groups require parent info (!parent_watched case) or the parent is * interested in this event. */ if (!parent_watched || (mask & p_mask & ALL_FSNOTIFY_EVENTS)) { ... } > struct name_snapshot name; > > /* When notifying parent, child should be passed as data */ > @@ -346,15 +374,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_type, > (!child || !child->i_fsnotify_marks)) > return 0; > > - /* An event "on child" is not intended for a mount/sb mark */ > - marks_mask = to_tell->i_fsnotify_mask; > - if (!child) { > - marks_mask |= sb->s_fsnotify_mask; > - if (mnt) > - marks_mask |= mnt->mnt_fsnotify_mask; > - } else { > + marks_mask = to_tell->i_fsnotify_mask | sb->s_fsnotify_mask; > + if (mnt) > + marks_mask |= mnt->mnt_fsnotify_mask; > + if (child) > marks_mask |= child->i_fsnotify_mask; > - } This hunk seems to belong to the previous patch... It fixes the problem I've spotted there. Honza
On Tue, Jul 14, 2020 at 2:54 PM Jan Kara <jack@suse.cz> wrote: > > On Thu 02-07-20 15:57:38, Amir Goldstein wrote: > > Similar to events "on child" to watching directory, send event "on child" > > with parent/name info if sb/mount/non-dir marks are interested in > > parent/name info. > > > > The FS_EVENT_ON_CHILD flag can be set on sb/mount/non-dir marks to specify > > interest in parent/name info for events on non-directory inodes. > > > > Events on "orphan" children (disconnected dentries) are sent without > > parent/name info. > > > > Events on direcories are send with parent/name info only if the parent > > directory is watching. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/notify/fsnotify.c | 50 +++++++++++++++++++++++--------- > > include/linux/fsnotify.h | 10 +++++-- > > include/linux/fsnotify_backend.h | 32 +++++++++++++++++--- > > 3 files changed, 73 insertions(+), 19 deletions(-) > > > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > > index 7c6e624b24c9..6683c77a5b13 100644 > > --- a/fs/notify/fsnotify.c > > +++ b/fs/notify/fsnotify.c > > @@ -144,27 +144,55 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode) > > > > /* > > * Notify this dentry's parent about a child's events with child name info > > - * if parent is watching. > > - * Notify only the child without name info if parent is not watching. > > + * if parent is watching or if inode/sb/mount are interested in events with > > + * parent and name info. > > + * > > + * Notify only the child without name info if parent is not watching and > > + * inode/sb/mount are not interested in events with parent and name info. > > */ > > int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, > > int data_type) > > { > > Ugh, I have to say this function is now pretty difficult to digest. I was > staring at it for an hour before I could make some sence of it... > Yeh, it took many trials to get to something that does the job, but it could use some cosmetic changes ;-) > > + const struct path *path = fsnotify_data_path(data, data_type); > > + struct mount *mnt = path ? real_mount(path->mnt) : NULL; > > struct inode *inode = d_inode(dentry); > > struct dentry *parent; > > + bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; > > + __u32 p_mask, test_mask, marks_mask = 0; > > struct inode *p_inode; > > int ret = 0; > > > > + /* > > + * Do inode/sb/mount care about parent and name info on non-dir? > > + * Do they care about any event at all? > > + */ > > + if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && > > + (!mnt || !mnt->mnt_fsnotify_marks)) { > > + if (!parent_watched) > > + return 0; > > + } else if (!(mask & FS_ISDIR) && !IS_ROOT(dentry)) { > > + marks_mask |= fsnotify_want_parent(inode->i_fsnotify_mask); > > + marks_mask |= fsnotify_want_parent(inode->i_sb->s_fsnotify_mask); > > + if (mnt) > > + marks_mask |= fsnotify_want_parent(mnt->mnt_fsnotify_mask); > > + } > > OK, so AFAIU at this point (mask & marks_mask) tells us whether we need to > grab parent because some mark needs parent into reported. Correct? > > Maybe I'd rename fsnotify_want_parent() (which seems like it returns bool) > to fsnotify_parent_needed_mask() or something like that. Also I'd hide all > those checks in a helper function like: > > fsnotify_event_needs_parent(mnt, inode, mask) > > So we'd then have just something like: > if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && > (!mnt || !mnt->mnt_fsnotify_marks) && !parent_watched) > return 0; > if (!parent_watched && !fsnotify_event_needs_parent(mnt, inode, mask)) > goto notify_child; OK, but we'd still need to store marks_mask for later in case event needs parent. > ... > > > + > > parent = NULL; > > - if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) > > + test_mask = mask & FS_EVENTS_POSS_TO_PARENT; > > + if (!(marks_mask & test_mask) && !parent_watched) > > goto notify_child; > > > > + /* Does parent inode care about events on children? */ > > parent = dget_parent(dentry); > > p_inode = parent->d_inode; > > + p_mask = fsnotify_inode_watches_children(p_inode); > > > > - if (unlikely(!fsnotify_inode_watches_children(p_inode))) { > > + if (p_mask) > > + marks_mask |= p_mask; > > + else if (unlikely(parent_watched)) > > __fsnotify_update_child_dentry_flags(p_inode); > > - } else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) { > > + > > + if ((marks_mask & test_mask) && p_inode != inode) { > ^^ this is effectively > !IS_ROOT(dentry), isn't it? But since you've checked that above can it ever > be that p_inode == inode? > True, but only if you add the hidden assumption (which should be true) that DCACHE_FSNOTIFY_PARENT_WATCHED cannot be set on an IS_ROOT dentry. Otherwise you can have parent_watched and not check IS_ROOT() so prefered defensive here, but I don't mind dropping it. > Also if marks_mask & test_mask == 0, why do you go to a branch that > notifies child? There shouldn't be anything to report there either. Am I > missing something? ... Oh, I see, the FS_EVENTS_POSS_TO_PARENT masking > above could cause that child is still interested. Because mask & marks_mask > can still contain something. Maybe it would be more comprehensible if we > restructure the above like: > > p_mask = fsnotify_inode_watches_children(p_inode); > if (unlikely(parent_watched && !p_mask)) > __fsnotify_update_child_dentry_flags(p_inode); > /* > * Include parent in notification either if some notification > * groups require parent info (!parent_watched case) or the parent is > * interested in this event. > */ > if (!parent_watched || (mask & p_mask & ALL_FSNOTIFY_EVENTS)) { > ... > } > Sure! > > struct name_snapshot name; > > > > /* When notifying parent, child should be passed as data */ > > @@ -346,15 +374,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_type, > > (!child || !child->i_fsnotify_marks)) > > return 0; > > > > - /* An event "on child" is not intended for a mount/sb mark */ > > - marks_mask = to_tell->i_fsnotify_mask; > > - if (!child) { > > - marks_mask |= sb->s_fsnotify_mask; > > - if (mnt) > > - marks_mask |= mnt->mnt_fsnotify_mask; > > - } else { > > + marks_mask = to_tell->i_fsnotify_mask | sb->s_fsnotify_mask; > > + if (mnt) > > + marks_mask |= mnt->mnt_fsnotify_mask; > > + if (child) > > marks_mask |= child->i_fsnotify_mask; > > - } > > This hunk seems to belong to the previous patch... It fixes the problem > I've spotted there. I figured that we would find it here :-) I will fix the previous patch. Thanks, Amir.
> > > + const struct path *path = fsnotify_data_path(data, data_type); > > > + struct mount *mnt = path ? real_mount(path->mnt) : NULL; > > > struct inode *inode = d_inode(dentry); > > > struct dentry *parent; > > > + bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; > > > + __u32 p_mask, test_mask, marks_mask = 0; > > > struct inode *p_inode; > > > int ret = 0; > > > > > > + /* > > > + * Do inode/sb/mount care about parent and name info on non-dir? > > > + * Do they care about any event at all? > > > + */ > > > + if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && > > > + (!mnt || !mnt->mnt_fsnotify_marks)) { > > > + if (!parent_watched) > > > + return 0; > > > + } else if (!(mask & FS_ISDIR) && !IS_ROOT(dentry)) { > > > + marks_mask |= fsnotify_want_parent(inode->i_fsnotify_mask); > > > + marks_mask |= fsnotify_want_parent(inode->i_sb->s_fsnotify_mask); > > > + if (mnt) > > > + marks_mask |= fsnotify_want_parent(mnt->mnt_fsnotify_mask); > > > + } > > > > OK, so AFAIU at this point (mask & marks_mask) tells us whether we need to > > grab parent because some mark needs parent into reported. Correct? > > > > Maybe I'd rename fsnotify_want_parent() (which seems like it returns bool) > > to fsnotify_parent_needed_mask() or something like that. Also I'd hide all > > those checks in a helper function like: > > > > fsnotify_event_needs_parent(mnt, inode, mask) > > > > So we'd then have just something like: > > if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && > > (!mnt || !mnt->mnt_fsnotify_marks) && !parent_watched) > > return 0; > > if (!parent_watched && !fsnotify_event_needs_parent(mnt, inode, mask)) > > goto notify_child; > > OK, but we'd still need to store marks_mask for later in case event > needs parent. > Nevermind, I understand now what you meant. Changed the function according to your guidance and looks much better. Fits now in one terminal screen :-) > > > ... > > > > > + > > > parent = NULL; > > > - if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) > > > + test_mask = mask & FS_EVENTS_POSS_TO_PARENT; > > > + if (!(marks_mask & test_mask) && !parent_watched) > > > goto notify_child; > > > > > > + /* Does parent inode care about events on children? */ > > > parent = dget_parent(dentry); > > > p_inode = parent->d_inode; > > > + p_mask = fsnotify_inode_watches_children(p_inode); > > > > > > - if (unlikely(!fsnotify_inode_watches_children(p_inode))) { > > > + if (p_mask) > > > + marks_mask |= p_mask; > > > + else if (unlikely(parent_watched)) > > > __fsnotify_update_child_dentry_flags(p_inode); > > > - } else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) { > > > + > > > + if ((marks_mask & test_mask) && p_inode != inode) { > > ^^ this is effectively > > !IS_ROOT(dentry), isn't it? But since you've checked that above can it ever > > be that p_inode == inode? > > > > True, but only if you add the hidden assumption (which should be true) that > DCACHE_FSNOTIFY_PARENT_WATCHED cannot be set on an IS_ROOT > dentry. Otherwise you can have parent_watched and not check IS_ROOT() > so prefered defensive here, but I don't mind dropping it. > I dropped this and also dropped the IS_ROOT(dentry) check because there is already a check in the inline fsnotify_parent(). FYI, pushed these changes to branch fsnotify_name. Rebased/tested/pushed branch fanotify_name_fid on top. There are no changes to following patches as they are all fanotify patches. Thanks, Amir.
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 7c6e624b24c9..6683c77a5b13 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -144,27 +144,55 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode) /* * Notify this dentry's parent about a child's events with child name info - * if parent is watching. - * Notify only the child without name info if parent is not watching. + * if parent is watching or if inode/sb/mount are interested in events with + * parent and name info. + * + * Notify only the child without name info if parent is not watching and + * inode/sb/mount are not interested in events with parent and name info. */ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, int data_type) { + const struct path *path = fsnotify_data_path(data, data_type); + struct mount *mnt = path ? real_mount(path->mnt) : NULL; struct inode *inode = d_inode(dentry); struct dentry *parent; + bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; + __u32 p_mask, test_mask, marks_mask = 0; struct inode *p_inode; int ret = 0; + /* + * Do inode/sb/mount care about parent and name info on non-dir? + * Do they care about any event at all? + */ + if (!inode->i_fsnotify_marks && !inode->i_sb->s_fsnotify_marks && + (!mnt || !mnt->mnt_fsnotify_marks)) { + if (!parent_watched) + return 0; + } else if (!(mask & FS_ISDIR) && !IS_ROOT(dentry)) { + marks_mask |= fsnotify_want_parent(inode->i_fsnotify_mask); + marks_mask |= fsnotify_want_parent(inode->i_sb->s_fsnotify_mask); + if (mnt) + marks_mask |= fsnotify_want_parent(mnt->mnt_fsnotify_mask); + } + parent = NULL; - if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) + test_mask = mask & FS_EVENTS_POSS_TO_PARENT; + if (!(marks_mask & test_mask) && !parent_watched) goto notify_child; + /* Does parent inode care about events on children? */ parent = dget_parent(dentry); p_inode = parent->d_inode; + p_mask = fsnotify_inode_watches_children(p_inode); - if (unlikely(!fsnotify_inode_watches_children(p_inode))) { + if (p_mask) + marks_mask |= p_mask; + else if (unlikely(parent_watched)) __fsnotify_update_child_dentry_flags(p_inode); - } else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) { + + if ((marks_mask & test_mask) && p_inode != inode) { struct name_snapshot name; /* When notifying parent, child should be passed as data */ @@ -346,15 +374,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_type, (!child || !child->i_fsnotify_marks)) return 0; - /* An event "on child" is not intended for a mount/sb mark */ - marks_mask = to_tell->i_fsnotify_mask; - if (!child) { - marks_mask |= sb->s_fsnotify_mask; - if (mnt) - marks_mask |= mnt->mnt_fsnotify_mask; - } else { + marks_mask = to_tell->i_fsnotify_mask | sb->s_fsnotify_mask; + if (mnt) + marks_mask |= mnt->mnt_fsnotify_mask; + if (child) marks_mask |= child->i_fsnotify_mask; - } /* * if this is a modify event we may need to clear the ignored masks diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 9b2566d273a9..044cae3a0628 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -44,10 +44,16 @@ static inline int fsnotify_parent(struct dentry *dentry, __u32 mask, { struct inode *inode = d_inode(dentry); - if (S_ISDIR(inode->i_mode)) + if (S_ISDIR(inode->i_mode)) { mask |= FS_ISDIR; - if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) + /* sb/mount marks are not interested in name of directory */ + if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) + goto notify_child; + } + + /* disconnected dentry cannot notify parent */ + if (IS_ROOT(dentry)) goto notify_child; return __fsnotify_parent(dentry, mask, data, data_type); diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 2c62628566c5..a7363c33211e 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -49,8 +49,11 @@ #define FS_OPEN_EXEC_PERM 0x00040000 /* open/exec event in a permission hook */ #define FS_EXCL_UNLINK 0x04000000 /* do not send events if object is unlinked */ -/* This inode cares about things that happen to its children. Always set for - * dnotify and inotify. */ +/* + * Set on inode mark that cares about things that happen to its children. + * Always set for dnotify and inotify. + * Set on inode/sb/mount marks that care about parenet/name info. + */ #define FS_EVENT_ON_CHILD 0x08000000 #define FS_DN_RENAME 0x10000000 /* file renamed */ @@ -72,14 +75,22 @@ FS_OPEN_EXEC_PERM) /* - * This is a list of all events that may get sent to a parent based on fs event - * happening to inodes inside that directory. + * This is a list of all events that may get sent to a parent that is watching + * with flag FS_EVENT_ON_CHILD based on fs event on a child of that directory. */ #define FS_EVENTS_POSS_ON_CHILD (ALL_FSNOTIFY_PERM_EVENTS | \ FS_ACCESS | FS_MODIFY | FS_ATTRIB | \ FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | \ FS_OPEN | FS_OPEN_EXEC) +/* + * This is a list of all events that may get sent with the parent inode as the + * @to_tell argument of fsnotify(). + * It may include events that can be sent to an inode/sb/mount mark, but cannot + * be sent to a parent watching children. + */ +#define FS_EVENTS_POSS_TO_PARENT (FS_EVENTS_POSS_ON_CHILD) + /* Events that can be reported to backends */ #define ALL_FSNOTIFY_EVENTS (ALL_FSNOTIFY_DIRENT_EVENTS | \ FS_EVENTS_POSS_ON_CHILD | \ @@ -398,6 +409,19 @@ extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt); extern void fsnotify_sb_delete(struct super_block *sb); extern u32 fsnotify_get_cookie(void); +static inline __u32 fsnotify_want_parent(__u32 mask) +{ + /* FS_EVENT_ON_CHILD is set on marks that want parent/name info */ + if (!(mask & FS_EVENT_ON_CHILD)) + return 0; + /* + * This object might be watched by a mark that cares about parent/name + * info, does it care about the specific set of events that can be + * reported with parent/name info? + */ + return mask & FS_EVENTS_POSS_TO_PARENT; +} + static inline int fsnotify_inode_watches_children(struct inode *inode) { /* FS_EVENT_ON_CHILD is set if the inode may care */
Similar to events "on child" to watching directory, send event "on child" with parent/name info if sb/mount/non-dir marks are interested in parent/name info. The FS_EVENT_ON_CHILD flag can be set on sb/mount/non-dir marks to specify interest in parent/name info for events on non-directory inodes. Events on "orphan" children (disconnected dentries) are sent without parent/name info. Events on direcories are send with parent/name info only if the parent directory is watching. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/notify/fsnotify.c | 50 +++++++++++++++++++++++--------- include/linux/fsnotify.h | 10 +++++-- include/linux/fsnotify_backend.h | 32 +++++++++++++++++--- 3 files changed, 73 insertions(+), 19 deletions(-)