From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] linux-2.6.18/gnttab: add deferred freeing logic Date: Thu, 15 Mar 2012 15:00:18 -0400 Message-ID: <20120315190018.GA3486@phenom.dumpdata.com> References: <4F5A1DE202000078000775EE@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <4F5A1DE202000078000775EE@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On Fri, Mar 09, 2012 at 02:12:34PM +0000, Jan Beulich wrote: > Signed-off-by: Jan Beulich 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 > #include > #include > +#include > #include > #include > #include > @@ -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 > > --- a/drivers/xen/core/gnttab.c > +++ b/drivers/xen/core/gnttab.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -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