| Message ID | d35d1067-a6f6-09be-9c4c-3e08a3bec3be@sandeen.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I suppose this should be titled "... on log processing" It happens during the log handling part of mount, but is outside of dirty log replay. On 3/7/18 5:32 PM, Eric Sandeen wrote: > Process any unlinked inodes unconditionally; this allows us to > skip dirtying the log on frozen filesystems and still have > proper recovery on the next mount. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 1937a93..2a645c0 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -5854,8 +5854,6 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) > */ > xfs_log_force(log->l_mp, XFS_LOG_SYNC); > > - xlog_recover_process_iunlinks(log); > - > xlog_recover_check_summary(log); > > xfs_notice(log->l_mp, "Ending recovery (logdev: %s)", > @@ -5865,6 +5863,14 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) > } else { > xfs_info(log->l_mp, "Ending clean mount"); > } > + > + /* > + * Process any unlinked inodes unconditionally, this allows us to > + * skip dirtying the log on frozen filesystems and still have > + * proper recovery on the next mount. > + */ > + xlog_recover_process_iunlinks(log); > + > return 0; > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 07, 2018 at 05:32:29PM -0600, Eric Sandeen wrote: > Process any unlinked inodes unconditionally; this allows us to > skip dirtying the log on frozen filesystems and still have > proper recovery on the next mount. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 1937a93..2a645c0 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -5854,8 +5854,6 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) > */ > xfs_log_force(log->l_mp, XFS_LOG_SYNC); > > - xlog_recover_process_iunlinks(log); > - > xlog_recover_check_summary(log); > > xfs_notice(log->l_mp, "Ending recovery (logdev: %s)", > @@ -5865,6 +5863,14 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) > } else { > xfs_info(log->l_mp, "Ending clean mount"); > } > + > + /* > + * Process any unlinked inodes unconditionally, this allows us to > + * skip dirtying the log on frozen filesystems and still have > + * proper recovery on the next mount. > + */ > + xlog_recover_process_iunlinks(log); > + The code seems fine. The only nit I have is maybe we should pull down the "Ending ..." messages until after the iunlinks call. That aside, this does introduce the same kind of mount delay we've been batting around wrt to the agfl padding fixup thing. We already have the scan in some places (perag reservation init calculations, cow blocks scan), some of which may be able to be removed/replaced going forward. I think this one falls into the latter category as well, but I'd be fine with this for the time being so long as the benefit is valuable enough. Have we considered anything like conditionally dirtying the log on freeze only when there are open+unlinked files? It seems like that may be uncommon enough to address the problem for snapshot users (particularly the read-only use case mentioned in the cover letter), but that's just a guess. Brian > return 0; > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/15/18 8:17 AM, Brian Foster wrote: > Have we considered anything like conditionally dirtying the log on > freeze only when there are open+unlinked files? It seems like that may > be uncommon enough to address the problem for snapshot users > (particularly the read-only use case mentioned in the cover letter), but > that's just a guess. > > Brian I did consider that, and was weighing the advantages with the disadvantages, namely unpredictable behavior for snapshots... I'm not sure how uncommon the situation really is. Regarding the mount delay, Darrick suggested that maybe we need to get all these scans into one place if possible, for efficiency. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 15, 2018 at 08:19:40AM -0400, Eric Sandeen wrote: > > > On 3/15/18 8:17 AM, Brian Foster wrote: > > Have we considered anything like conditionally dirtying the log on > > freeze only when there are open+unlinked files? It seems like that may > > be uncommon enough to address the problem for snapshot users > > (particularly the read-only use case mentioned in the cover letter), but > > that's just a guess. > > > > Brian > > > I did consider that, and was weighing the advantages with the disadvantages, > namely unpredictable behavior for snapshots... I'm not sure how uncommon > the situation really is. > Ok. > Regarding the mount delay, Darrick suggested that maybe we need to get > all these scans into one place if possible, for efficiency. > These buffers should be cached after the first scan, right? As Dave mentioned on one of the previous threads, I think the I/O load is more of a factor than anything else. I'd rather see us try to eliminate these kinds of scans rather than condense them into some kind of infrastructure that tries to self-justify their existence. But as previously mentioned, I think it's reasonable to add this one for now and optimize it away separately. Brian > -Eric > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 1937a93..2a645c0 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -5854,8 +5854,6 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) */ xfs_log_force(log->l_mp, XFS_LOG_SYNC); - xlog_recover_process_iunlinks(log); - xlog_recover_check_summary(log); xfs_notice(log->l_mp, "Ending recovery (logdev: %s)", @@ -5865,6 +5863,14 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) } else { xfs_info(log->l_mp, "Ending clean mount"); } + + /* + * Process any unlinked inodes unconditionally, this allows us to + * skip dirtying the log on frozen filesystems and still have + * proper recovery on the next mount. + */ + xlog_recover_process_iunlinks(log); + return 0; }
Process any unlinked inodes unconditionally; this allows us to skip dirtying the log on frozen filesystems and still have proper recovery on the next mount. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html