From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH] linux-2.6.18/gnttab: add deferred freeing logic
Date: Thu, 15 Mar 2012 15:00:18 -0400 [thread overview]
Message-ID: <20120315190018.GA3486@phenom.dumpdata.com> (raw)
In-Reply-To: <4F5A1DE202000078000775EE@nat28.tlf.novell.com>
On Fri, Mar 09, 2012 at 02:12:34PM +0000, Jan Beulich wrote:
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Nice. any plans of adding something similar to pvops?
Was there a BZ that triggered having an timer doing this?
>
> --- a/drivers/xen/core/gnttab.c
> +++ b/drivers/xen/core/gnttab.c
> @@ -35,6 +35,7 @@
> #include <linux/sched.h>
> #include <linux/mm.h>
> #include <linux/seqlock.h>
> +#include <linux/timer.h>
> #include <xen/interface/xen.h>
> #include <xen/gnttab.h>
> #include <asm/pgtable.h>
> @@ -183,35 +184,119 @@ int gnttab_query_foreign_access(grant_re
> }
> EXPORT_SYMBOL_GPL(gnttab_query_foreign_access);
>
> -int gnttab_end_foreign_access_ref(grant_ref_t ref)
> +static inline int _gnttab_end_foreign_access_ref(grant_ref_t ref)
> {
> u16 flags, nflags;
>
> nflags = shared[ref].flags;
> do {
> - if ((flags = nflags) & (GTF_reading|GTF_writing)) {
> - printk(KERN_DEBUG "WARNING: g.e. still in use!\n");
> + if ((flags = nflags) & (GTF_reading|GTF_writing))
> return 0;
> - }
> } while ((nflags = synch_cmpxchg_subword(&shared[ref].flags, flags, 0)) !=
> flags);
>
> return 1;
> }
> +
> +int gnttab_end_foreign_access_ref(grant_ref_t ref)
> +{
> + if (_gnttab_end_foreign_access_ref(ref))
> + return 1;
> + printk(KERN_DEBUG "WARNING: g.e. %#x still in use!\n", ref);
> + return 0;
> +}
> EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref);
>
> +struct deferred_entry {
> + struct list_head list;
> + grant_ref_t ref;
> + uint16_t warn_delay;
> + struct page *page;
> +};
> +static LIST_HEAD(deferred_list);
> +static void gnttab_handle_deferred(unsigned long);
> +static DEFINE_TIMER(deferred_timer, gnttab_handle_deferred, 0, 0);
> +
> +static void gnttab_handle_deferred(unsigned long unused)
> +{
> + unsigned int nr = 10;
> + struct deferred_entry *first = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + while (nr--) {
> + struct deferred_entry *entry
> + = list_first_entry(&deferred_list,
> + struct deferred_entry, list);
> +
> + if (entry == first)
> + break;
> + list_del(&entry->list);
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> + if (_gnttab_end_foreign_access_ref(entry->ref)) {
> + put_free_entry(entry->ref);
> + if (entry->page) {
> + printk(KERN_DEBUG
> + "freeing g.e. %#x (pfn %#lx)\n",
> + entry->ref, page_to_pfn(entry->page));
> + __free_page(entry->page);
> + } else
> + printk(KERN_DEBUG "freeing g.e. %#x\n",
> + entry->ref);
> + kfree(entry);
> + entry = NULL;
> + } else {
> + if (!--entry->warn_delay)
> + printk(KERN_INFO "g.e. %#x still pending\n",
> + entry->ref);
> + if (!first)
> + first = entry;
> + }
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + if (entry)
> + list_add_tail(&entry->list, &deferred_list);
> + else if (list_empty(&deferred_list))
> + break;
> + }
> + if (!list_empty(&deferred_list) && !timer_pending(&deferred_timer)) {
> + deferred_timer.expires = jiffies + HZ;
> + add_timer(&deferred_timer);
> + }
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> +}
> +
> +static void gnttab_add_deferred(grant_ref_t ref, struct page *page)
> +{
> + struct deferred_entry *entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
> + const char *what = KERN_WARNING "leaking";
> +
> + if (entry) {
> + unsigned long flags;
> +
> + entry->ref = ref;
> + entry->page = page;
> + entry->warn_delay = 60;
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + list_add_tail(&entry->list, &deferred_list);
> + if (!timer_pending(&deferred_timer)) {
> + deferred_timer.expires = jiffies + HZ;
> + add_timer(&deferred_timer);
> + }
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> + what = KERN_DEBUG "deferring";
> + }
> + printk("%s g.e. %#x (pfn %lx)\n", what,
> + ref, page ? page_to_pfn(page) : -1);
> +}
> +
> void gnttab_end_foreign_access(grant_ref_t ref, unsigned long page)
> {
> if (gnttab_end_foreign_access_ref(ref)) {
> put_free_entry(ref);
> if (page != 0)
> free_page(page);
> - } else {
> - /* XXX This needs to be fixed so that the ref and page are
> - placed on a list to be freed up later. */
> - printk(KERN_DEBUG
> - "WARNING: leaking g.e. and page still in use!\n");
> - }
> + } else
> + gnttab_add_deferred(ref, page ? virt_to_page(page) : NULL);
> }
> EXPORT_SYMBOL_GPL(gnttab_end_foreign_access);
>
>
>
> gnttab: add deferred freeing logic
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/drivers/xen/core/gnttab.c
> +++ b/drivers/xen/core/gnttab.c
> @@ -35,6 +35,7 @@
> #include <linux/sched.h>
> #include <linux/mm.h>
> #include <linux/seqlock.h>
> +#include <linux/timer.h>
> #include <xen/interface/xen.h>
> #include <xen/gnttab.h>
> #include <asm/pgtable.h>
> @@ -183,35 +184,119 @@ int gnttab_query_foreign_access(grant_re
> }
> EXPORT_SYMBOL_GPL(gnttab_query_foreign_access);
>
> -int gnttab_end_foreign_access_ref(grant_ref_t ref)
> +static inline int _gnttab_end_foreign_access_ref(grant_ref_t ref)
> {
> u16 flags, nflags;
>
> nflags = shared[ref].flags;
> do {
> - if ((flags = nflags) & (GTF_reading|GTF_writing)) {
> - printk(KERN_DEBUG "WARNING: g.e. still in use!\n");
> + if ((flags = nflags) & (GTF_reading|GTF_writing))
> return 0;
> - }
> } while ((nflags = synch_cmpxchg_subword(&shared[ref].flags, flags, 0)) !=
> flags);
>
> return 1;
> }
> +
> +int gnttab_end_foreign_access_ref(grant_ref_t ref)
> +{
> + if (_gnttab_end_foreign_access_ref(ref))
> + return 1;
> + printk(KERN_DEBUG "WARNING: g.e. %#x still in use!\n", ref);
> + return 0;
> +}
> EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref);
>
> +struct deferred_entry {
> + struct list_head list;
> + grant_ref_t ref;
> + uint16_t warn_delay;
> + struct page *page;
> +};
> +static LIST_HEAD(deferred_list);
> +static void gnttab_handle_deferred(unsigned long);
> +static DEFINE_TIMER(deferred_timer, gnttab_handle_deferred, 0, 0);
> +
> +static void gnttab_handle_deferred(unsigned long unused)
> +{
> + unsigned int nr = 10;
> + struct deferred_entry *first = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + while (nr--) {
> + struct deferred_entry *entry
> + = list_first_entry(&deferred_list,
> + struct deferred_entry, list);
> +
> + if (entry == first)
> + break;
> + list_del(&entry->list);
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> + if (_gnttab_end_foreign_access_ref(entry->ref)) {
> + put_free_entry(entry->ref);
> + if (entry->page) {
> + printk(KERN_DEBUG
> + "freeing g.e. %#x (pfn %#lx)\n",
> + entry->ref, page_to_pfn(entry->page));
> + __free_page(entry->page);
> + } else
> + printk(KERN_DEBUG "freeing g.e. %#x\n",
> + entry->ref);
> + kfree(entry);
> + entry = NULL;
> + } else {
> + if (!--entry->warn_delay)
> + printk(KERN_INFO "g.e. %#x still pending\n",
> + entry->ref);
> + if (!first)
> + first = entry;
> + }
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + if (entry)
> + list_add_tail(&entry->list, &deferred_list);
> + else if (list_empty(&deferred_list))
> + break;
> + }
> + if (!list_empty(&deferred_list) && !timer_pending(&deferred_timer)) {
> + deferred_timer.expires = jiffies + HZ;
> + add_timer(&deferred_timer);
> + }
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> +}
> +
> +static void gnttab_add_deferred(grant_ref_t ref, struct page *page)
> +{
> + struct deferred_entry *entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
> + const char *what = KERN_WARNING "leaking";
> +
> + if (entry) {
> + unsigned long flags;
> +
> + entry->ref = ref;
> + entry->page = page;
> + entry->warn_delay = 60;
> + spin_lock_irqsave(&gnttab_list_lock, flags);
> + list_add_tail(&entry->list, &deferred_list);
> + if (!timer_pending(&deferred_timer)) {
> + deferred_timer.expires = jiffies + HZ;
> + add_timer(&deferred_timer);
> + }
> + spin_unlock_irqrestore(&gnttab_list_lock, flags);
> + what = KERN_DEBUG "deferring";
> + }
> + printk("%s g.e. %#x (pfn %lx)\n", what,
> + ref, page ? page_to_pfn(page) : -1);
> +}
> +
> void gnttab_end_foreign_access(grant_ref_t ref, unsigned long page)
> {
> if (gnttab_end_foreign_access_ref(ref)) {
> put_free_entry(ref);
> if (page != 0)
> free_page(page);
> - } else {
> - /* XXX This needs to be fixed so that the ref and page are
> - placed on a list to be freed up later. */
> - printk(KERN_DEBUG
> - "WARNING: leaking g.e. and page still in use!\n");
> - }
> + } else
> + gnttab_add_deferred(ref, page ? virt_to_page(page) : NULL);
> }
> EXPORT_SYMBOL_GPL(gnttab_end_foreign_access);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2012-03-15 19:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-09 14:12 [PATCH] linux-2.6.18/gnttab: add deferred freeing logic Jan Beulich
2012-03-15 19:00 ` Konrad Rzeszutek Wilk [this message]
2012-03-16 8:15 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120315190018.GA3486@phenom.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).