| Message ID | 1530591079-33813-2-git-send-email-yang.shi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 03, 2018 at 12:11:19PM +0800, Yang Shi wrote: > direct reclaim doesn't write out filesystem page, only kswapd could do > this. So, if it is called from direct relaim, it is definitely a bug. > > And, Mel Gorman mentioned "Ultimately, this will be a BUG_ON." in commit > 94054fa3fca1fd78db02cb3d68d5627120f0a1d4 ("xfs: warn if direct reclaim > tries to writeback pages"), > > It has been many years since that commit, so it should be safe to > elevate WARN_ON to BUG_ON now. NACK. The existing code warns and then handles the situation gracefully - this is the appropriate way to handle incorrect calling contexts. There is absolutely no good reason to panic production kernels in situations like this. Cheers, Dave.
On Tue, Jul 03, 2018 at 12:11:19PM +0800, Yang Shi wrote: > direct reclaim doesn't write out filesystem page, only kswapd could do > this. So, if it is called from direct relaim, it is definitely a bug. > > And, Mel Gorman mentioned "Ultimately, this will be a BUG_ON." in commit > 94054fa3fca1fd78db02cb3d68d5627120f0a1d4 ("xfs: warn if direct reclaim > tries to writeback pages"), > > It has been many years since that commit, so it should be safe to > elevate WARN_ON to BUG_ON now. > > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Darrick J. Wong <darrick.wong@oracle.com> > Cc: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com> > --- > fs/xfs/xfs_aops.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 8eb3ba3..7efc2d2 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -1080,11 +1080,9 @@ static inline int xfs_bio_add_buffer(struct bio *bio, struct buffer_head *bh) > * allow reclaim from kswapd as the stack usage there is relatively low. > * > * This should never happen except in the case of a VM regression so > - * warn about it. > + * BUG about it. > */ > - if (WARN_ON_ONCE((current->flags & (PF_MEMALLOC|PF_KSWAPD)) == > - PF_MEMALLOC)) > - goto redirty; > + BUG_ON((current->flags & (PF_MEMALLOC|PF_KSWAPD)) == PF_MEMALLOC); Ugh, please do not increase the BUG() factor. Even if this happens due to a regression it's /much/ easier to debug if we don't halt the system. (IOWs, I decline to take this patch.) --D > > /* > * Given that we do not allow direct reclaim to call us, we should > -- > 1.8.3.1 > > -- > 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_aops.c b/fs/xfs/xfs_aops.c index 8eb3ba3..7efc2d2 100644 --- a/fs/xfs/xfs_aops.c +++ b/fs/xfs/xfs_aops.c @@ -1080,11 +1080,9 @@ static inline int xfs_bio_add_buffer(struct bio *bio, struct buffer_head *bh) * allow reclaim from kswapd as the stack usage there is relatively low. * * This should never happen except in the case of a VM regression so - * warn about it. + * BUG about it. */ - if (WARN_ON_ONCE((current->flags & (PF_MEMALLOC|PF_KSWAPD)) == - PF_MEMALLOC)) - goto redirty; + BUG_ON((current->flags & (PF_MEMALLOC|PF_KSWAPD)) == PF_MEMALLOC); /* * Given that we do not allow direct reclaim to call us, we should
direct reclaim doesn't write out filesystem page, only kswapd could do this. So, if it is called from direct relaim, it is definitely a bug. And, Mel Gorman mentioned "Ultimately, this will be a BUG_ON." in commit 94054fa3fca1fd78db02cb3d68d5627120f0a1d4 ("xfs: warn if direct reclaim tries to writeback pages"), It has been many years since that commit, so it should be safe to elevate WARN_ON to BUG_ON now. Cc: Mel Gorman <mgorman@techsingularity.net> Cc: Darrick J. Wong <darrick.wong@oracle.com> Cc: Dave Chinner <dchinner@redhat.com> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com> --- fs/xfs/xfs_aops.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)