Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1584014 > unrolled thread
| Started by | James Simmons <jsimmons@infradead.org> |
|---|---|
| First post | 2017-02-18 23:00 +0100 |
| Last post | 2017-02-25 05:10 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 13/14] staging: lustre: llog: limit file size of plain logs James Simmons <jsimmons@infradead.org> - 2017-02-18 23:00 +0100
Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-02-24 18:00 +0100
Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs Oleg Drokin <oleg.drokin@intel.com> - 2017-02-25 05:10 +0100
Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs Oleg Drokin <oleg.drokin@intel.com> - 2017-02-25 05:10 +0100
| From | James Simmons <jsimmons@infradead.org> |
|---|---|
| Date | 2017-02-18 23:00 +0100 |
| Subject | [PATCH 13/14] staging: lustre: llog: limit file size of plain logs |
| Message-ID | <tclcS-7hi-33@gated-at.bofh.it> |
From: Alex Zhuravlev <alexey.zhuravlev@intel.com>
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 <free space>/64 (128MB at most) at llog creation
time.
Signed-off-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
Reviewed-on: https://review.whamcloud.com/18028
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: wangdi <di.wang@intel.com>
Reviewed-by: Mike Pershin <mike.pershin@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
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;
+ }
+
CDEBUG(D_OTHER, "Re-read last llog buffer for new records, index %u, last %u\n",
index, loghandle->lgh_last_idx);
/* save offset inside buffer for the re-read */
--
1.8.3.1
[toc] | [next] | [standalone]
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-02-24 18:00 +0100 |
| Subject | Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs |
| Message-ID | <ternP-2WZ-15@gated-at.bofh.it> |
| In reply to | #1584014 |
On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
> From: Alex Zhuravlev <alexey.zhuravlev@intel.com>
>
> 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 <free space>/64 (128MB at most) at llog creation
> time.
>
> Signed-off-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
> Reviewed-on: https://review.whamcloud.com/18028
> Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
> Reviewed-by: wangdi <di.wang@intel.com>
> Reviewed-by: Mike Pershin <mike.pershin@intel.com>
> Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
> 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
[toc] | [prev] | [next] | [standalone]
| From | Oleg Drokin <oleg.drokin@intel.com> |
|---|---|
| Date | 2017-02-25 05:10 +0100 |
| Message-ID | <teBQd-2fx-1@gated-at.bofh.it> |
| In reply to | #1587796 |
On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev <alexey.zhuravlev@intel.com>
>>
>> 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 <free space>/64 (128MB at most) at llog creation
>> time.
>>
>> Signed-off-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
>> Reviewed-by: wangdi <di.wang@intel.com>
>> Reviewed-by: Mike Pershin <mike.pershin@intel.com>
>> Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
>> Signed-off-by: James Simmons <jsimmons@infradead.org>
>> ---
>> 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…
Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.
[toc] | [prev] | [next] | [standalone]
| From | Oleg Drokin <oleg.drokin@intel.com> |
|---|---|
| Date | 2017-02-25 05:10 +0100 |
| Message-ID | <teBQd-2fx-3@gated-at.bofh.it> |
| In reply to | #1587796 |
On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev <alexey.zhuravlev@intel.com>
>>
>> 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 <free space>/64 (128MB at most) at llog creation
>> time.
>>
>> Signed-off-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
>> Reviewed-by: wangdi <di.wang@intel.com>
>> Reviewed-by: Mike Pershin <mike.pershin@intel.com>
>> Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
>> Signed-off-by: James Simmons <jsimmons@infradead.org>
>> ---
>> 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…
Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web