From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751346AbdBXQ70 (ORCPT ); Fri, 24 Feb 2017 11:59:26 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:35852 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751219AbdBXQ7S (ORCPT ); Fri, 24 Feb 2017 11:59:18 -0500 Date: Fri, 24 Feb 2017 17:59:10 +0100 From: Greg Kroah-Hartman To: James Simmons Cc: devel@driverdev.osuosl.org, Andreas Dilger , Oleg Drokin , Alex Zhuravlev , Linux Kernel Mailing List , Lustre Development List Subject: Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs Message-ID: <20170224165910.GB9990@kroah.com> References: <1487454435-4895-1-git-send-email-jsimmons@infradead.org> <1487454435-4895-14-git-send-email-jsimmons@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1487454435-4895-14-git-send-email-jsimmons@infradead.org> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote: > From: Alex Zhuravlev > > on small filesystems plain log can grow dramatically. especially > given large record sizes produced by DNE and extended chunksize. > I saw >50% of space consumed by a single llog file which was still > in use. this leads to test failures (sanityn, etc). > the patch introduces additional limit on plain llog size, which > is calculated as /64 (128MB at most) at llog creation > time. > > Signed-off-by: Alex Zhuravlev > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838 > Reviewed-on: https://review.whamcloud.com/18028 > Reviewed-by: Andreas Dilger > Reviewed-by: wangdi > Reviewed-by: Mike Pershin > Reviewed-by: Oleg Drokin > Signed-off-by: James Simmons > --- > drivers/staging/lustre/lustre/obdclass/llog.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/staging/lustre/lustre/obdclass/llog.c b/drivers/staging/lustre/lustre/obdclass/llog.c > index 83c5b62..320ff6b 100644 > --- a/drivers/staging/lustre/lustre/obdclass/llog.c > +++ b/drivers/staging/lustre/lustre/obdclass/llog.c > @@ -319,10 +319,26 @@ static int llog_process_thread(void *arg) > * the case and re-read the current chunk > * otherwise. > */ > + int records; > + > if (index > loghandle->lgh_last_idx) { > rc = 0; > goto out; > } > + /* <2 records means no more records > + * if the last record we processed was > + * the final one, then the underlying > + * object might have been destroyed yet. > + * we better don't access that.. > + */ > + mutex_lock(&loghandle->lgh_hdr_mutex); > + records = loghandle->lgh_hdr->llh_count; > + mutex_unlock(&loghandle->lgh_hdr_mutex); > + if (records <= 1) { > + rc = 0; > + goto out; > + } So you now use the lock, in only one place, when reading a single value? That makes no sense, it's obviously wrong, or not needed. Please fix up these two patches... thanks, greg k-h