| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <91071BA5-593A-40C8-8525-84CBBC8BB883@intel.com>
Date: Sun, 16 Oct 2016 13:16:44 -0400
From: Oleg Drokin <oleg.drokin@...el.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: James Simmons <jsimmons@...radead.org>,
<devel@...verdev.osuosl.org>,
Andreas Dilger <andreas.dilger@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Jinshan Xiong <jinshan.xiong@...el.com>,
Lustre Development List <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH 57/80] staging: lustre: osc: revise unstable pages accounting
On Oct 16, 2016, at 11:14 AM, Greg Kroah-Hartman wrote:
> Digging up an old email...
>
> On Tue, Aug 16, 2016 at 04:19:10PM -0400, James Simmons wrote:
>> From: Jinshan Xiong <jinshan.xiong@...el.com>
>>
>> A few changes are made in this patch for unstable pages tracking:
>>
>> 1. Remove kernel NFS unstable pages tracking because it killed
>> performance
>> 2. Track unstable pages as part of LRU cache. Otherwise Lustre
>> can use much more memory than max_cached_mb
>> 3. Remove obd_unstable_pages tracking to avoid using global
>> atomic counter
>> 4. Make unstable pages track optional. Tracking unstable pages is
>> turned off by default, and can be controlled by
>> llite.*.unstable_stats.
>>
>> Signed-off-by: Jinshan Xiong <jinshan.xiong@...el.com>
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4841
>> Reviewed-on: http://review.whamcloud.com/10003
>> Reviewed-by: Andreas Dilger <andreas.dilger@...el.com>
>> Reviewed-by: Lai Siyao <lai.siyao@...el.com>
>> Reviewed-by: Oleg Drokin <oleg.drokin@...el.com>
>> Signed-off-by: James Simmons <jsimmons@...radead.org>
>> ---
>> drivers/staging/lustre/lustre/include/cl_object.h | 35 +++-
>> .../staging/lustre/lustre/include/obd_support.h | 1 -
>> drivers/staging/lustre/lustre/llite/lproc_llite.c | 41 ++++-
>> drivers/staging/lustre/lustre/obdclass/class_obd.c | 2 -
>> drivers/staging/lustre/lustre/osc/osc_cache.c | 96 +---------
>> drivers/staging/lustre/lustre/osc/osc_internal.h | 2 +-
>> drivers/staging/lustre/lustre/osc/osc_page.c | 208 +++++++++++++++++---
>> drivers/staging/lustre/lustre/osc/osc_request.c | 13 +-
>> 8 files changed, 253 insertions(+), 145 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/include/cl_object.h b/drivers/staging/lustre/lustre/include/cl_object.h
>> index d269b32..ec6cf7c 100644
>> --- a/drivers/staging/lustre/lustre/include/cl_object.h
>> +++ b/drivers/staging/lustre/lustre/include/cl_object.h
>> @@ -1039,23 +1039,32 @@ do { \
>> } \
>> } while (0)
>>
>> -static inline int __page_in_use(const struct cl_page *page, int refc)
>> -{
>> - if (page->cp_type == CPT_CACHEABLE)
>> - ++refc;
>> - LASSERT(atomic_read(&page->cp_ref) > 0);
>> - return (atomic_read(&page->cp_ref) > refc);
>> -}
>> -
>> -#define cl_page_in_use(pg) __page_in_use(pg, 1)
>> -#define cl_page_in_use_noref(pg) __page_in_use(pg, 0)
>> -
>> static inline struct page *cl_page_vmpage(struct cl_page *page)
>> {
>> LASSERT(page->cp_vmpage);
>> return page->cp_vmpage;
>> }
>>
>> +/**
>> + * Check if a cl_page is in use.
>> + *
>> + * Client cache holds a refcount, this refcount will be dropped when
>> + * the page is taken out of cache, see vvp_page_delete().
>> + */
>> +static inline bool __page_in_use(const struct cl_page *page, int refc)
>> +{
>> + return (atomic_read(&page->cp_ref) > refc + 1);
>> +}
>> +
>> +/**
>> + * Caller itself holds a refcount of cl_page.
>> + */
>> +#define cl_page_in_use(pg) __page_in_use(pg, 1)
>> +/**
>> + * Caller doesn't hold a refcount.
>> + */
>> +#define cl_page_in_use_noref(pg) __page_in_use(pg, 0)
>> +
>> /** @} cl_page */
>>
>> /** \addtogroup cl_lock cl_lock
>> @@ -2331,6 +2340,10 @@ struct cl_client_cache {
>> */
>> spinlock_t ccc_lru_lock;
>> /**
>> + * Set if unstable check is enabled
>> + */
>> + unsigned int ccc_unstable_check:1;
>> + /**
>> * # of unstable pages for this mount point
>> */
>> atomic_t ccc_unstable_nr;
>> diff --git a/drivers/staging/lustre/lustre/include/obd_support.h b/drivers/staging/lustre/lustre/include/obd_support.h
>> index 26fdff6..a11fff1 100644
>> --- a/drivers/staging/lustre/lustre/include/obd_support.h
>> +++ b/drivers/staging/lustre/lustre/include/obd_support.h
>> @@ -54,7 +54,6 @@ extern int at_early_margin;
>> extern int at_extra;
>> extern unsigned int obd_sync_filter;
>> extern unsigned int obd_max_dirty_pages;
>> -extern atomic_t obd_unstable_pages;
>> extern atomic_t obd_dirty_pages;
>> extern atomic_t obd_dirty_transit_pages;
>> extern char obd_jobid_var[];
>> diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
>> index 2f1f389..5f8e78d 100644
>> --- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
>> +++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
>> @@ -828,10 +828,45 @@ static ssize_t unstable_stats_show(struct kobject *kobj,
>> pages = atomic_read(&cache->ccc_unstable_nr);
>> mb = (pages * PAGE_SIZE) >> 20;
>>
>> - return sprintf(buf, "unstable_pages: %8d\n"
>> - "unstable_mb: %8d\n", pages, mb);
>> + return sprintf(buf, "unstable_check: %8d\n"
>> + "unstable_pages: %8d\n"
>> + "unstable_mb: %8d\n",
>> + cache->ccc_unstable_check, pages, mb);
>> }
>> -LUSTRE_RO_ATTR(unstable_stats);
>> +
>> +static ssize_t unstable_stats_store(struct kobject *kobj,
>> + struct attribute *attr,
>> + const char *buffer,
>> + size_t count)
>> +{
>> + struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
>> + ll_kobj);
>> + char kernbuf[128];
>> + int val, rc;
>> +
>> + if (!count)
>> + return 0;
>> + if (count < 0 || count >= sizeof(kernbuf))
>> + return -EINVAL;
>> +
>> + if (copy_from_user(kernbuf, buffer, count))
>> + return -EFAULT;
>
> It was just pointed out to me that this code has obviously never been
> tested at all.
Whoops.
> Sorry for missing this before, do you want me to revert this? Or will
> you send me a fix?
I'll have a fix for you in a moment.
> Also, go fix your test harness, it's not being used, or is totally buggy :)
Well, at least there was no crash, right? ;)
Powered by blists - more mailing lists