Re: [PATCH] dma-buf: Add DmaBufTotal counter in meminfo
On Fri, Apr 16, 2021 at 11:37:19AM +0200, Peter Enderborg wrote: > diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c > index 6fa761c9cc78..3c1a82b51a6f 100644 > --- a/fs/proc/meminfo.c > +++ b/fs/proc/meminfo.c > @@ -16,6 +16,7 @@ > #ifdef CONFIG_CMA > #include > #endif > +#include > #include > #include "internal.h" > > @@ -145,6 +146,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) > show_val_kb(m, "CmaFree:", > global_zone_page_state(NR_FREE_CMA_PAGES)); > #endif > + show_val_kb(m, "DmaBufTotal:", dma_buf_get_size()); > > hugetlb_report_meminfo(m); > ... and if CONFIG_DMA_SHARED_BUFFER is not set ...? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all with drm_modeset_lock
On Thu, Apr 22, 2021 at 05:59:51PM +0200, Fabio M. De Francesco wrote:
> - drm_modeset_lock_all(drm_dev);
> -
> drm_for_each_crtc(crtc, drm_dev) {
> + drm_modeset_lock(&crtc->mutex, NULL);
> if (crtc->state->active) {
> ret = -EBUSY;
> - break;
> }
> + drm_modeset_unlock(&crtc->mutex);
> + if (ret < 0)
> + break;
> }
>
> - drm_modeset_unlock_all(drm_dev);
> -
I might remove the {} around ret = -EBUSY, but this is good.
Reviewed-by: Matthew Wilcox (Oracle)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/9] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks
On Fri, Aug 13, 2021 at 06:51:05PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 13, 2021 at 09:17:11AM -0600, Jim Cromie wrote:
> > +int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
> > +{
> > + unsigned long inbits;
> > + int rc, i, chgct = 0, totct = 0;
> > + char query[OUR_QUERY_SIZE];
> > + struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data;
>
> So you need space after ')' ?
More importantly, if ->data is of type 'void *', it is bad style to
cast the pointer at all. I can't tell what type 'data' has; if it
is added to kernel_param as part of this series, I wasn't cc'd on
the patch that did that.
Re: [PATCH] drm/ttm: stop warning on TT shrinker failure
On Mon, Mar 22, 2021 at 02:49:27PM +0100, Daniel Vetter wrote:
> On Sun, Mar 21, 2021 at 03:18:28PM +0100, Christian König wrote:
> > Am 20.03.21 um 14:17 schrieb Daniel Vetter:
> > > On Sat, Mar 20, 2021 at 10:04 AM Christian König
> > > wrote:
> > > > Am 19.03.21 um 20:06 schrieb Daniel Vetter:
> > > > > On Fri, Mar 19, 2021 at 07:53:48PM +0100, Christian König wrote:
> > > > > > Am 19.03.21 um 18:52 schrieb Daniel Vetter:
> > > > > > > On Fri, Mar 19, 2021 at 03:08:57PM +0100, Christian König wrote:
> > > > > > > > Don't print a warning when we fail to allocate a page for
> > > > > > > > swapping things out.
> > > > > > > >
> > > > > > > > Also rely on memalloc_nofs_save/memalloc_nofs_restore instead
> > > > > > > > of GFP_NOFS.
> > > > > > > Uh this part doesn't make sense. Especially since you only do it
> > > > > > > for the
> > > > > > > debugfs file, not in general. Which means you've just completely
> > > > > > > broken
> > > > > > > the shrinker.
> > > > > > Are you sure? My impression is that GFP_NOFS should now work much
> > > > > > more out
> > > > > > of the box with the memalloc_nofs_save()/memalloc_nofs_restore().
> > > > > Yeah, if you'd put it in the right place :-)
> > > > >
> > > > > But also -mm folks are very clear that memalloc_no*() family is for
> > > > > dire
> > > > > situation where there's really no other way out. For anything where
> > > > > you
> > > > > know what you're doing, you really should use explicit gfp flags.
> > > > My impression is just the other way around. You should try to avoid the
> > > > NOFS/NOIO flags and use the memalloc_no* approach instead.
> > > Where did you get that idea?
> >
> > Well from the kernel comment on GFP_NOFS:
> >
> > * %GFP_NOFS will use direct reclaim but will not use any filesystem
> > interfaces.
> > * Please try to avoid using this flag directly and instead use
> > * memalloc_nofs_{save,restore} to mark the whole scope which
> > cannot/shouldn't
> > * recurse into the FS layer with a short explanation why. All allocation
> > * requests will inherit GFP_NOFS implicitly.
>
> Huh that's interesting, since iirc Willy or Dave told me the opposite, and
> the memalloc_no* stuff is for e.g. nfs calling into network layer (needs
> GFP_NOFS) or swap on top of a filesystems (even needs GFP_NOIO I think).
>
> Adding them, maybe I got confused.
My impression is that the scoped API is preferred these days.
https://www.kernel.org/doc/html/latest/core-api/gfp_mask-from-fs-io.html
I'd probably need to spend a few months learning the DRM subsystem to
have a more detailed opinion on whether passing GFP flags around explicitly
or using the scope API is the better approach for your situation.
I usually defer to Michal on these kinds of questions.
> > > The kernel is full of explicit gfp_t flag
> > > passing to make this as explicit as possible. The memalloc_no* stuff
> > > is just for when you go through entire subsystems and really can't
> > > wire it through. I can't find the discussion anymore, but that was the
> > > advice I got from mm/fs people.
> > >
> > > One reason is that generally a small GFP_KERNEL allocation never
> > > fails. But it absolutely can fail if it's in a memalloc_no* section,
> > > and these kind of non-obvious non-local effects are a real pain in
> > > testing and review. Hence explicit gfp_flag passing as much as
> > > possible.
I agree with this; it's definitely a problem with the scope API. I wanted
to extend it to include GFP_NOWAIT, but if you do that, your chances of
memory allocation failure go way up, so you really want to set __GFP_NOWARN
too, but now you need to audit all the places that you're calling to be
sure they really handle errors correctly.
So I think I'm giving up on that patch set.
> > > > > > > If this is just to paper over the seq_printf doing the wrong
> > > > > > > allocations,
> > > > > > > then just move that out from under the fs_reclaim_acquire/release
> > > > > > > part.
> > > > > > No, that wasn't the problem.
> > > > > >
> > > > > > We have just seen to many failures to allocate pages for swapout
> > > > > > and I think
> > > > > > that would improve this because in a lot of cases we can then
> > > > > > immediately
> > > > > > swap things out instead of having to rely on upper layers.
> > > > > Yeah, you broke it. Now the real shrinker is running with GFP_KERNEL,
> > > > > because your memalloc_no is only around the debugfs function. And ofc
> > > > > it's
> > > > > much easier to allocate with GFP_KERNEL, right until you deadlock :-)
> > > > The problem here is that for example kswapd calls the shrinker without
> > > > holding a FS lock as far as I can see.
> > > >
> > > > And it is rather sad that we can't optimize this case directly.
> > > I'm still not clear what you want to optimize? You can check for "is
> > > this kswapd" in pf flags, but that sounds very hairy and fragile.
> >
> > Well we only need the NOFS flag when the shrinker callback really comes f
Re: [PATCH 1/2] mm: replace BUG_ON in vm_insert_page with a return of an error
On Tue, Feb 02, 2021 at 04:31:33PM -0800, Suren Baghdasaryan wrote: > Replace BUG_ON(vma->vm_flags & VM_PFNMAP) in vm_insert_page with > WARN_ON_ONCE and returning an error. This is to ensure users of the > vm_insert_page that set VM_PFNMAP are notified of the wrong flag usage > and get an indication of an error without panicing the kernel. > This will help identifying drivers that need to clear VM_PFNMAP before > using dmabuf system heap which is moving to use vm_insert_page. NACK. The system may not _panic_, but it is clearly now _broken_. The device doesn't work, and so the system is useless. You haven't really improved anything here. Just bloated the kernel with yet another _ONCE variable that in a normal system will never ever ever be triggered. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] dmabuf: Add dmabuf inode number to /proc/*/fdinfo
On Fri, Feb 05, 2021 at 02:23:20AM +, Kalesh Singh wrote: > +DMA Buffer files > + > + > +:: > + > + pos:0 > + flags: 04002 > + mnt_id: 9 > + dmabuf_inode_no: 63107 inode_nr would be better than inode_no. But even better would be to match stat(2) and just call it 'ino'. > + size: 32768 > + count: 2 > + exp_name: system-heap ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/8] mm: Remove special swap entry functions
On Tue, Mar 09, 2021 at 11:14:58PM +1100, Alistair Popple wrote:
> -static inline struct page *migration_entry_to_page(swp_entry_t entry)
> -{
> - struct page *p = pfn_to_page(swp_offset(entry));
> - /*
> - * Any use of migration entries may only occur while the
> - * corresponding page is locked
> - */
> - BUG_ON(!PageLocked(compound_head(p)));
> - return p;
> -}
> +static inline struct page *pfn_swap_entry_to_page(swp_entry_t entry)
> +{
> + struct page *p = pfn_to_page(swp_offset(entry));
> +
> + /*
> + * Any use of migration entries may only occur while the
> + * corresponding page is locked
> + */
> + BUG_ON(is_migration_entry(entry) && !PageLocked(compound_head(p)));
> +
> + return p;
> +}
I appreciate you're only moving this code, but PageLocked includes an
implicit compound_head():
1. __PAGEFLAG(Locked, locked, PF_NO_TAIL)
2. #define __PAGEFLAG(uname, lname, policy)\
TESTPAGEFLAG(uname, lname, policy) \
3. #define TESTPAGEFLAG(uname, lname, policy) \
static __always_inline int Page##uname(struct page *page) \
{ return test_bit(PG_##lname, &policy(page, 0)->flags); }
4. #define PF_NO_TAIL(page, enforce) ({\
VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page); \
PF_POISONED_CHECK(compound_head(page)); })
5. #define PF_POISONED_CHECK(page) ({ \
VM_BUG_ON_PGFLAGS(PagePoisoned(page), page);\
page; })
This macrology isn't easy to understand the first time you read it (nor,
indeed, the tenth time), so let me decode it:
Substitute 5 into 4 and remove irrelevancies:
6. #define PF_NO_TAIL(page, enforce) compound_head(page)
Expand 1 in 2:
7. TESTPAGEFLAG(Locked, locked, PF_NO_TAIL)
Expand 7 in 3:
8. static __always_inline int PageLocked(struct page *page)
{ return test_bit(PG_locked, &PF_NO_TAIL(page, 0)->flags); }
Expand 6 in 8:
9. static __always_inline int PageLocked(struct page *page)
{ return test_bit(PG_locked, &compound_head(page)->flags); }
(in case it's not clear, compound_head() is idempotent. that is:
f(f(a)) == f(a))
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: make alloc_anon_inode more useful
On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> this series first renames the existing alloc_anon_inode to
> alloc_anon_inode_sb to clearly mark it as requiring a superblock.
>
> It then adds a new alloc_anon_inode that works on the anon_inode
> file system super block, thus removing tons of boilerplate code.
>
> The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> later, but might also be ripe for some cleanup.
On a somewhat related note, could I get you to look at
drivers/video/fbdev/core/fb_defio.c?
As far as I can tell, there's no need for fb_deferred_io_aops to exist.
We could just set file->f_mapping->a_ops to NULL, and set_page_dirty()
would do the exact same thing this code does (except it would get the
return value correct).
But maybe that would make something else go wrong that distinguishes
between page->mapping being NULL and page->mapping->a_ops->foo being NULL?
Completely untested patch ...
diff --git a/drivers/video/fbdev/core/fb_defio.c
b/drivers/video/fbdev/core/fb_defio.c
index a591d291b231..441ec31d3e4d 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -151,17 +151,6 @@ static const struct vm_operations_struct
fb_deferred_io_vm_ops = {
.page_mkwrite = fb_deferred_io_mkwrite,
};
-static int fb_deferred_io_set_page_dirty(struct page *page)
-{
- if (!PageDirty(page))
- SetPageDirty(page);
- return 0;
-}
-
-static const struct address_space_operations fb_deferred_io_aops = {
- .set_page_dirty = fb_deferred_io_set_page_dirty,
-};
-
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
vma->vm_ops = &fb_deferred_io_vm_ops;
@@ -212,14 +201,6 @@ void fb_deferred_io_init(struct fb_info *info)
}
EXPORT_SYMBOL_GPL(fb_deferred_io_init);
-void fb_deferred_io_open(struct fb_info *info,
-struct inode *inode,
-struct file *file)
-{
- file->f_mapping->a_ops = &fb_deferred_io_aops;
-}
-EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-
void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 06f5805de2de..c4ba76359f22 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1415,10 +1415,7 @@ __releases(&info->lock)
if (res)
module_put(info->fbops->owner);
}
-#ifdef CONFIG_FB_DEFERRED_IO
- if (info->fbdefio)
- fb_deferred_io_open(info, inode, file);
-#endif
+ file->f_mapping->a_ops = NULL;
out:
unlock_fb_info(info);
if (res)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ecfbcc0553a5..a8dccd23c249 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -659,9 +659,6 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32
d_pitch,
/* drivers/video/fb_defio.c */
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
extern void fb_deferred_io_init(struct fb_info *info);
-extern void fb_deferred_io_open(struct fb_info *info,
- struct inode *inode,
- struct file *file);
extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb_defio: Remove custom address_space_operations
On Wed, Mar 10, 2021 at 06:38:07PM +, William Kucharski wrote:
> Looks good, just one super minor nit inline.
> > @@ -228,13 +202,6 @@ void fb_deferred_io_cleanup(struct fb_info *info)
> >
> > BUG_ON(!fbdefio);
> > cancel_delayed_work_sync(&info->deferred_work);
> > -
> > - /* clear out the mapping that we setup */
> > - for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
> > - page = fb_deferred_io_page(info, i);
> > - page->mapping = NULL;
> > - }
> > -
> > mutex_destroy(&fbdefio->lock);
> > }
>
> We no longer need the definition of "int i" right before the BUG_ON().
Huh. Usually gcc warns about that. let me figure that out and post a v2.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On Mon, May 10, 2021 at 02:16:16PM +0100, Edward Cree wrote:
> On 10/05/2021 12:55, Mauro Carvalho Chehab wrote:
> > The main point on this series is to replace just the occurrences
> > where ASCII represents the symbol equally well
>
> > - U+2014 ('—'): EM DASH
> Em dash is not the same thing as hyphen-minus, and the latter does not
> serve 'equally well'. People use em dashes because — even in
> monospace fonts — they make text easier to read and comprehend, when
> used correctly.
> I accept that some of the other distinctions — like en dashes — are
> needlessly pedantic (though I don't doubt there is someone out there
> who will gladly defend them with the same fervour with which I argue
> for the em dash) and I wouldn't take the trouble to use them myself;
> but I think there is a reasonable assumption that when someone goes
> to the effort of using a Unicode punctuation mark that is semantic
> (rather than merely typographical), they probably had a reason for
> doing so.
I think you're overestimating the amount of care and typographical
knowledge that your average kernel developer has. Most of these
UTF-8 characters come from latex conversions and really aren't
necessary (and are being used incorrectly).
You seem quite knowedgeable about the various differences. Perhaps
you'd be willing to write a document for Documentation/doc-guide/
that provides guidance for when to use which kinds of horizontal
line? https://www.punctuationmatters.com/hyphen-dash-n-dash-and-m-dash/
talks about it in the context of publications, but I think we need
something more suited to our needs for kernel documentation.
Re: [PATCH] drivers/video/fbdev/core/fbmem.c: add pointer judgment
On Wed, May 19, 2021 at 08:00:28PM +0800, songqiang wrote: > Signed-off-by: songqiang > --- You need to explain: - Why you think this patch is needed - Did you observe a problem at runtime? - Is this the output from some checking tool? - Why this is the right way to address the problem
Re: [PATCH v2 1/8] mm: slab: provide krealloc_array()
On Mon, Nov 02, 2020 at 04:20:30PM +0100, Bartosz Golaszewski wrote: > +Chunks allocated with `kmalloc` can be resized with `krealloc`. Similarly > +to `kmalloc_array`: a helper for resising arrays is provided in the form of > +`krealloc_array`. Is there any reason you chose to `do_this` instead of do_this()? The automarkup script turns do_this() into a nice link to the documentation which you're adding below. Typo 'resising' resizing. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/3] mm: split vm_normal_pages for LRU and non-LRU handling
On Thu, Mar 10, 2022 at 11:26:31AM -0600, Alex Sierra wrote:
> @@ -606,7 +606,7 @@ static void print_bad_pte(struct vm_area_struct *vma,
> unsigned long addr,
> * PFNMAP mappings in order to support COWable mappings.
> *
> */
> -struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
> +struct page *vm_normal_any_page(struct vm_area_struct *vma, unsigned long
> addr,
> pte_t pte)
> {
> unsigned long pfn = pte_pfn(pte);
> @@ -620,8 +620,6 @@ struct page *vm_normal_page(struct vm_area_struct *vma,
> unsigned long addr,
> return NULL;
> if (is_zero_pfn(pfn))
> return NULL;
> - if (pte_devmap(pte))
> - return NULL;
>
> print_bad_pte(vma, addr, pte, NULL);
> return NULL;
... what?
Haven't you just made it so that a devmap page always prints a bad PTE
message, and then returns NULL anyway?
Surely this should be:
if (pte_devmap(pte))
- return NULL;
+ return pfn_to_page(pfn);
or maybe
+ goto check_pfn;
But I don't know about that highest_memmap_pfn check.
> @@ -661,6 +659,22 @@ struct page *vm_normal_page(struct vm_area_struct *vma,
> unsigned long addr,
> return pfn_to_page(pfn);
> }
>
> +/*
> + * vm_normal_lru_page -- This function gets the "struct page" associated
> + * with a pte only for page cache and anon page. These pages are LRU handled.
> + */
> +struct page *vm_normal_lru_page(struct vm_area_struct *vma, unsigned long
> addr,
> + pte_t pte)
It seems a shame to add a new function without proper kernel-doc.
Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range
On Fri, Feb 25, 2022 at 06:42:37PM +, Tvrtko Ursulin wrote: > Matthew, what do you think fix for this build warning on h8300 and s390 > should be? Or perhaps a build environment issue with kernel test robot? I'd suggest this should do the job: +++ b/include/linux/cacheflush.h @@ -4,6 +4,8 @@ #include +struct folio; + #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_FOLIO void flush_dcache_folio(struct folio *folio);
Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote: > We can do > > typeof(pos) pos > > in the 'for ()' loop, and never use __iter at all. > > That means that inside the for-loop, we use a _different_ 'pos' than outside. Then we can never use -Wshadow ;-( I'd love to be able to turn it on; it catches real bugs. > +#define list_for_each_entry(pos, head, member) > \ > + for (typeof(pos) pos = list_first_entry(head, typeof(*pos), member); > \ > + !list_entry_is_head(pos, head, member);\ >pos = list_next_entry(pos, member))
Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
On Mon, Feb 28, 2022 at 12:37:15PM -0800, Linus Torvalds wrote:
> On Mon, Feb 28, 2022 at 12:16 PM Matthew Wilcox wrote:
> >
> > Then we can never use -Wshadow ;-( I'd love to be able to turn it on;
> > it catches real bugs.
>
> Oh, we already can never use -Wshadow regardless of things like this.
> That bridge hasn't just been burned, it never existed in the first
> place.
>
> The whole '-Wshadow' thing simply cannot work with local variables in
> macros - something that we've used since day 1.
>
> Try this (as a "p.c" file):
>
> #define min(a,b) ({ \
> typeof(a) __a = (a);\
> typeof(b) __b = (b);\
> __a < __b ? __a : __b; })
>
> int min3(int a, int b, int c)
> {
> return min(a,min(b,c));
> }
>
> and now do "gcc -O2 -S t.c".
>
> Then try it with -Wshadow.
#define ___PASTE(a, b) a##b
#define __PASTE(a, b) ___PASTE(a, b)
#define _min(a, b, u) ({ \
typeof(a) __PASTE(__a,u) = (a);\
typeof(b) __PASTE(__b,u) = (b);\
__PASTE(__a,u) < __PASTE(__b,u) ? __PASTE(__a,u) : __PASTE(__b,u); })
#define min(a, b) _min(a, b, __COUNTER__)
int min3(int a, int b, int c)
{
return min(a,min(b,c));
}
(probably there's a more elegant way to do this)
Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
On Tue, Mar 01, 2022 at 10:14:07AM -0800, Kees Cook wrote:
> On Mon, Feb 28, 2022 at 04:45:11PM -0800, Linus Torvalds wrote:
> > Really. The "-Wshadow doesn't work on the kernel" is not some new
> > issue, because you have to do completely insane things to the source
> > code to enable it.
>
> The first big glitch with -Wshadow was with shadowed global variables.
> GCC 4.8 fixed that, but it still yells about shadowed functions. What
> _almost_ works is -Wshadow=local. At first glace, all the warnings
> look solvable, but then one will eventually discover __wait_event()
> and associated macros that mix when and how deeply it intentionally
> shadows variables. :)
Well, that's just disgusting. Macros fundamentally shouldn't be
referring to things that aren't in their arguments. The first step to
cleaning this up is ...
I'll take a look at the rest of cleaning this up soon.
>From 28ffe35d56223d4242b915832299e5acc926737e Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)"
Date: Tue, 1 Mar 2022 13:47:07 -0500
Subject: [PATCH] wait: Parameterize the return variable to ___wait_event()
Macros should not refer to variables which aren't in their arguments.
Pass the name from its callers.
Signed-off-by: Matthew Wilcox (Oracle)
---
include/linux/swait.h| 12 ++--
include/linux/wait.h | 32
include/linux/wait_bit.h | 4 ++--
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/include/linux/swait.h b/include/linux/swait.h
index 6a8c22b8c2a5..5e8e9b13be2d 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -191,14 +191,14 @@ do {
\
} while (0)
#define __swait_event_timeout(wq, condition, timeout) \
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_UNINTERRUPTIBLE, timeout,\
__ret = schedule_timeout(__ret))
#define swait_event_timeout_exclusive(wq, condition, timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret))\
__ret = __swait_event_timeout(wq, condition, timeout); \
__ret; \
})
@@ -216,14 +216,14 @@ do {
\
})
#define __swait_event_interruptible_timeout(wq, condition, timeout)\
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_INTERRUPTIBLE, timeout, \
__ret = schedule_timeout(__ret))
#define swait_event_interruptible_timeout_exclusive(wq, condition, timeout)\
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret))\
__ret = __swait_event_interruptible_timeout(wq, \
condition, timeout);\
__ret; \
@@ -252,7 +252,7 @@ do {
\
} while (0)
#define __swait_event_idle_timeout(wq, condition, timeout) \
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_IDLE, timeout, \
__ret = schedule_timeout(__ret))
@@ -278,7 +278,7 @@ do {
\
#define swait_event_idle_timeout_exclusive(wq, condition, timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret))\
__ret = __swait_event_idle_timeout(wq, \
condition, timeout); \
__ret; \
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 851e07da2583..890cce3c0f2e 1006
Re: [PATCH RFC] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 04:15:33AM +0200, Jarkko Sakkinen wrote: > Sometimes you might want to use MAP_POPULATE to ask a device driver to > initialize the device memory in some specific manner. SGX driver can use > this to request more memory by issuing ENCLS[EAUG] x86 opcode for each > page in the address range. > > Add f_ops->populate() with the same parameters as f_ops->mmap() and make > it conditionally called inside call_mmap(). Update call sites > accodingly. Your device driver has a ->mmap operation. Why does it need another one? More explanation required here.
Re: [PATCH RFC] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 05:21:11AM +0200, Jarkko Sakkinen wrote: > On Sun, Mar 06, 2022 at 02:57:55AM +0000, Matthew Wilcox wrote: > > On Sun, Mar 06, 2022 at 04:15:33AM +0200, Jarkko Sakkinen wrote: > > > Sometimes you might want to use MAP_POPULATE to ask a device driver to > > > initialize the device memory in some specific manner. SGX driver can use > > > this to request more memory by issuing ENCLS[EAUG] x86 opcode for each > > > page in the address range. > > > > > > Add f_ops->populate() with the same parameters as f_ops->mmap() and make > > > it conditionally called inside call_mmap(). Update call sites > > > accodingly. > > > > Your device driver has a ->mmap operation. Why does it need another > > one? More explanation required here. > > f_ops->mmap() would require an additional parameter, which results > heavy refactoring. > > struct file_operations has 1125 references in the kernel tree, so I > decided to check this way around first. Are you saying that your device driver behaves differently if MAP_POPULATE is set versus if it isn't? That seems hideously broken.
Re: [PATCH RFC] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 06:11:21AM +0200, Jarkko Sakkinen wrote: > On Sun, Mar 06, 2022 at 03:52:12AM +0000, Matthew Wilcox wrote: > > On Sun, Mar 06, 2022 at 05:21:11AM +0200, Jarkko Sakkinen wrote: > > > On Sun, Mar 06, 2022 at 02:57:55AM +0000, Matthew Wilcox wrote: > > > > On Sun, Mar 06, 2022 at 04:15:33AM +0200, Jarkko Sakkinen wrote: > > > > > Sometimes you might want to use MAP_POPULATE to ask a device driver to > > > > > initialize the device memory in some specific manner. SGX driver can > > > > > use > > > > > this to request more memory by issuing ENCLS[EAUG] x86 opcode for each > > > > > page in the address range. > > > > > > > > > > Add f_ops->populate() with the same parameters as f_ops->mmap() and > > > > > make > > > > > it conditionally called inside call_mmap(). Update call sites > > > > > accodingly. > > > > > > > > Your device driver has a ->mmap operation. Why does it need another > > > > one? More explanation required here. > > > > > > f_ops->mmap() would require an additional parameter, which results > > > heavy refactoring. > > > > > > struct file_operations has 1125 references in the kernel tree, so I > > > decided to check this way around first. > > > > Are you saying that your device driver behaves differently if > > MAP_POPULATE is set versus if it isn't? That seems hideously broken. > > MAP_POPULATE does not do anything (according to __mm_populate in mm/gup.c) > with VMA's that have some sort of device/IO memory, i.e. vm_flags > intersecting with VM_PFNMAP | VM_IO. > > I can extend the guard obviously to: > > if (!ret && do_populate && file->f_op->populate && > !!(vma->vm_flags & (VM_IO | VM_PFNMAP))) > file->f_op->populate(file, vma); Are you deliberately avoiding the question? I'm not asking about implementation. I'm asking about the semantics of MAP_POPULATE with your driver.
Re: [PATCH RFC] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 06:25:52AM +0200, Jarkko Sakkinen wrote: > > Are you deliberately avoiding the question? I'm not asking about > > implementation. I'm asking about the semantics of MAP_POPULATE with > > your driver. > > No. I just noticed a bug in the guard from your comment that I wanted > point out. > > With the next version I post the corresponding change to the driver, > in order to see this in context. Oh, good grief. Don't bother. NAK.
Re: [PATCH RFC 0/3] MAP_POPULATE for device memory
On Sun, Mar 06, 2022 at 07:32:04AM +0200, Jarkko Sakkinen wrote: > For device memory (aka VM_IO | VM_PFNMAP) MAP_POPULATE does nothing. Allow > to use that for initializing the device memory by providing a new callback > f_ops->populate() for the purpose. As I said, NAK.
Re: [PATCH RFC 1/3] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 07:02:57PM +0200, Jarkko Sakkinen wrote: > So can I conclude from this that in general having populate available for > device memory is something horrid, or just the implementation path? You haven't even attempted to explain what the problem is you're trying to solve. You've shown up with some terrible code and said "Hey, is this a good idea". No, no, it's not.
Re: [PATCH RFC v2] mm: Add f_ops->populate()
On Sun, Mar 06, 2022 at 03:41:54PM -0800, Dave Hansen wrote: > In short: page faults stink. The core kernel has lots of ways of > avoiding page faults like madvise(MADV_WILLNEED) or mmap(MAP_POPULATE). > But, those only work on normal RAM that the core mm manages. > > SGX is weird. SGX memory is managed outside the core mm. It doesn't > have a 'struct page' and get_user_pages() doesn't work on it. Its VMAs > are marked with VM_IO. So, none of the existing methods for avoiding > page faults work on SGX memory. > > This essentially helps extend existing "normal RAM" kernel ABIs to work > for avoiding faults for SGX too. SGX users want to enjoy all of the > benefits of a delayed allocation policy (better resource use, > overcommit, NUMA affinity) but without the cost of millions of faults. We have a mechanism for dynamically reducing the number of page faults already; it's just buried in the page cache code. You have vma->vm_file, which contains a file_ra_state. You can use this to track where recent faults have been and grow the size of the region you fault in per page fault. You don't have to (indeed probably don't want to) use the same algorithm as the page cache, but the _principle_ is the same -- were recent speculative faults actually used; should we grow the number of pages actually faulted in, or is this a random sparse workload where we want to allocate individual pages. Don't rely on the user to ask. They don't know.
Re: [PATCH v1 00/12] MEMORY_DEVICE_COHERENT for CPU-accessible coherent device memory
On Tue, Oct 12, 2021 at 11:39:57AM -0700, Andrew Morton wrote: > Because I must ask: if this feature is for one single computer which > presumably has a custom kernel, why add it to mainline Linux? I think in particular patch 2 deserves to be merged because it removes a ton of cruft from every call to put_page() (at least if you're using a distro config). It makes me nervous, but I think it's the right thing to do. It may well need more fixups after it has been merged, but that's life.
Re: [PATCH v1 1/2] ext4/xfs: add page refcount helper
On Thu, Oct 14, 2021 at 10:39:27AM -0500, Alex Sierra wrote: > From: Ralph Campbell > > There are several places where ZONE_DEVICE struct pages assume a reference > count == 1 means the page is idle and free. Instead of open coding this, > add a helper function to hide this detail. > > Signed-off-by: Ralph Campbell > Signed-off-by: Alex Sierra > Reviewed-by: Christoph Hellwig > Acked-by: Theodore Ts'o > Acked-by: Darrick J. Wong Reviewed-by: Matthew Wilcox (Oracle)
Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
On Thu, Oct 14, 2021 at 10:39:28AM -0500, Alex Sierra wrote: > From: Ralph Campbell > > ZONE_DEVICE struct pages have an extra reference count that complicates the > code for put_page() and several places in the kernel that need to check the > reference count to see that a page is not being used (gup, compaction, > migration, etc.). Clean up the code so the reference count doesn't need to > be treated specially for ZONE_DEVICE. > > Signed-off-by: Ralph Campbell > Signed-off-by: Alex Sierra > Reviewed-by: Christoph Hellwig Reviewed-by: Matthew Wilcox (Oracle)
Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
It would probably help if you cc'd Dan on this.
As far as I know he's the only person left who cares about GUP on DAX.
On Thu, Oct 14, 2021 at 02:06:34PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 14, 2021 at 10:39:28AM -0500, Alex Sierra wrote:
> > From: Ralph Campbell
> >
> > ZONE_DEVICE struct pages have an extra reference count that complicates the
> > code for put_page() and several places in the kernel that need to check the
> > reference count to see that a page is not being used (gup, compaction,
> > migration, etc.). Clean up the code so the reference count doesn't need to
> > be treated specially for ZONE_DEVICE.
> >
> > Signed-off-by: Ralph Campbell
> > Signed-off-by: Alex Sierra
> > Reviewed-by: Christoph Hellwig
> > ---
> > v2:
> > AS: merged this patch in linux 5.11 version
> >
> > v5:
> > AS: add condition at try_grab_page to check for the zone device type, while
> > page ref counter is checked less/equal to zero. In case of device zone,
> > pages
> > ref counter are initialized to zero.
> >
> > v7:
> > AS: fix condition at try_grab_page added at v5, is invalid. It supposed
> > to fix xfstests/generic/413 test, however, there's a known issue on
> > this test where DAX mapped area DIO to non-DAX expect to fail.
> > https://patchwork.kernel.org/project/fstests/patch/1489463960-3579-1-git-send-email-xz...@redhat.com
> > This condition was removed after rebase over patch series
> > https://lore.kernel.org/r/20210813044133.1536842-4-jhubb...@nvidia.com
> > ---
> > arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +-
> > drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
> > fs/dax.c | 4 +-
> > include/linux/dax.h| 2 +-
> > include/linux/memremap.h | 7 +--
> > include/linux/mm.h | 11
> > lib/test_hmm.c | 2 +-
> > mm/internal.h | 8 +++
> > mm/memcontrol.c| 6 +--
> > mm/memremap.c | 69 +++---
> > mm/migrate.c | 5 --
> > mm/page_alloc.c| 3 ++
> > mm/swap.c | 45 ++---
> > 13 files changed, 46 insertions(+), 120 deletions(-)
>
> Has anyone tested this with FSDAX? Does get_user_pages() on fsdax
> backed memory still work?
>
> What refcount value does the struct pages have when they are installed
> in the PTEs? Remember a 0 refcount will make all the get_user_pages()
> fail.
>
> I'm looking at the call path starting in ext4_punch_hole() and I would
> expect to see something manipulating the page ref count before
> the ext4_break_layouts() call path gets to the dax_page_unused() test.
>
> All I see is we go into unmap_mapping_pages() - that would normally
> put back the page references held by PTEs but insert_pfn() has this:
>
> if (pfn_t_devmap(pfn))
> entry = pte_mkdevmap(pfn_t_pte(pfn, prot));
>
> And:
>
> static inline pte_t pte_mkdevmap(pte_t pte)
> {
> return pte_set_flags(pte, _PAGE_SPECIAL|_PAGE_DEVMAP);
> }
>
> Which interacts with vm_normal_page():
>
> if (pte_devmap(pte))
> return NULL;
>
> To disable that refcounting?
>
> So... I have a feeling this will have PTEs pointing to 0 refcount
> pages? Unless FSDAX is !pte_devmap which is not the case, right?
>
> This seems further confirmed by this comment:
>
> /*
>* If we race get_user_pages_fast() here either we'll see the
>* elevated page count in the iteration and wait, or
>* get_user_pages_fast() will see that the page it took a reference
>* against is no longer mapped in the page tables and bail to the
>* get_user_pages() slow path. The slow path is protected by
>* pte_lock() and pmd_lock(). New references are not taken without
>* holding those locks, and unmap_mapping_pages() will not zero the
>* pte or pmd without holding the respective lock, so we are
>* guaranteed to either see new references or prevent new
>* references from being established.
>*/
>
> Which seems to explain this scheme relies on unmap_mapping_pages() to
> fence GUP_fast, not on GUP_fast observing 0 refcounts when it should
> stop.
>
> This seems like it would be properly fixed by using normal page
> refcounting for PTEs - ie stop using special for these pages?
>
> Does anyone know why devmap is pte_special anyhow?
>
> > +void free_zone_device_page(struct page *page)
> > +{
> > + switch (page->pgmap->type) {
> > + case MEMORY_DEVICE_PRIVATE:
> > + free_device_page(page);
> > + return;
> > + case MEMORY_DEVICE_FS_DAX:
> > + /* notify page idle */
> > + wake_up_var(&page->_refcount);
> > + return;
>
> It is not for this series, but I wonder if we should just always call
> ops->page_free and have free_device_page() logic in that cal
Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
On Sat, Oct 16, 2021 at 12:44:50PM -0300, Jason Gunthorpe wrote: > Assuming changing FSDAX is hard.. How would DAX people feel about just > deleting the PUD/PMD support until it can be done with compound pages? I think there are customers who would find that an unacceptable answer :-)
Re: BoF at LPC: Documenting the Heterogeneous Memory Model Architecture
On Thu, Sep 23, 2021 at 04:25:08PM -0400, Felix Kuehling wrote: > Change of plan: Instead of a BoF, this is now a session in the "GPU/media/AI > buffer management and interop MC" micro conference. Thank you Daniel Stone > for making that happen. > https://linuxplumbersconf.org/event/11/contributions/1112/ > > It is scheduled for tomorrow (Friday) 08:40-10:00 Pacific, 11:40-13:00 > Eastern, 15:40-17:00 UTC. That's up against: Direct map management Vlastimil Babka, Mike Rapoport, Rick Edgecombe 11:30-12:15. Seems like a lot of the same people would want to be in both sessions. Maybe one could be moved?
Phyr Starter
TLDR: I want to introduce a new data type:
struct phyr {
phys_addr_t addr;
size_t len;
};
and use it to replace bio_vec as well as using it to replace the array
of struct pages used by get_user_pages() and friends.
---
There are two distinct problems I want to address: doing I/O to memory
which does not have a struct page and efficiently doing I/O to large
blobs of physically contiguous memory, regardless of whether it has a
struct page. There are some other improvements which I regard as minor.
There are many types of memory that one might want to do I/O to that do
not have a struct page, some examples:
- Memory on a graphics card (or other PCI card, but gfx seems to be
the primary provider of DRAM on the PCI bus today)
- DAX, or other pmem (there are some fake pages today, but this is
mostly a workaround for the IO problem today)
- Guest memory being accessed from the hypervisor (KVM needs to
create structpages to make this happen. Xen doesn't ...)
All of these kinds of memories can be addressed by the CPU and so also
by a bus master. That is, there is a physical address that the CPU
can use which will address this memory, and there is a way to convert
that to a DMA address which can be programmed into another device.
There's no intent here to support memory which can be accessed by a
complex scheme like writing an address to a control register and then
accessing the memory through a FIFO; this is for memory which can be
accessed by DMA and CPU loads and stores.
For get_user_pages() and friends, we currently fill an array of struct
pages, each one representing PAGE_SIZE bytes. For an application that
is using 1GB hugepages, writing 2^18 entries is a significant overhead.
It also makes drivers hard to write as they have to recoalesce the
struct pages, even though the VM can tell it whether those 2^18 pages
are contiguous.
On the minor side, struct phyr can represent any mappable chunk of memory.
A bio_vec is limited to 2^32 bytes, while on 64-bit machines a phyr
can represent larger than 4GB. A phyr is the same size as a bio_vec
on 64 bit (16 bytes), and the same size for 32-bit with PAE (12 bytes).
It is smaller for 32-bit machines without PAE (8 bytes instead of 12).
Finally, it may be possible to stop using scatterlist to describe the
input to the DMA-mapping operation. We may be able to get struct
scatterlist down to just dma_address and dma_length, with chaining
handled through an enclosing struct.
I would like to see phyr replace bio_vec everywhere it's currently used.
I don't have time to do that work now because I'm busy with folios.
If someone else wants to take that on, I shall cheer from the sidelines.
What I do intend to do is:
- Add an interface to gup.c to pin/unpin N phyrs
- Add a sg_map_phyrs()
This will take an array of phyrs and allocate an sg for them
- Whatever else I need to do to make one RDMA driver happy with
this scheme
At that point, I intend to stop and let others more familiar with this
area of the kernel continue the conversion of drivers.
P.S. If you've had the Prodigy song running through your head the whole
time you've been reading this email ... I'm sorry / You're welcome.
If people insist, we can rename this to phys_range or something boring,
but I quite like the spelling of phyr with the pronunciation of "fire".
Re: Phyr Starter
On Mon, Jan 10, 2022 at 08:41:26PM -0400, Jason Gunthorpe wrote: > On Mon, Jan 10, 2022 at 07:34:49PM +0000, Matthew Wilcox wrote: > > > Finally, it may be possible to stop using scatterlist to describe the > > input to the DMA-mapping operation. We may be able to get struct > > scatterlist down to just dma_address and dma_length, with chaining > > handled through an enclosing struct. > > Can you talk about this some more? IMHO one of the key properties of > the scatterlist is that it can hold huge amounts of pages without > having to do any kind of special allocation due to the chaining. > > The same will be true of the phyr idea right? My thinking is that we'd pass a relatively small array of phyr (maybe 16 entries) to get_user_phyr(). If that turned out not to be big enough, then we have two options; one is to map those 16 ranges with sg and use the sg chaining functionality before throwing away the phyr and calling get_user_phyr() again. The other is to stash those 16 ranges somewhere (eg a resizing array of some kind) and keep calling get_user_phyr() to get the next batch of 16; once we've got the entire range, call sg_map_phyr() passing all of the phyrs. > > I would like to see phyr replace bio_vec everywhere it's currently used. > > I don't have time to do that work now because I'm busy with folios. > > If someone else wants to take that on, I shall cheer from the sidelines. > > What I do intend to do is: > > I wonder if we mixed things though.. > > IMHO there is a lot of optimization to be had by having a > datastructure that is expressly 'the physical pages underlying a > contiguous chunk of va' > > If you limit to that scenario then we can be more optimal because > things like byte granular offsets and size in the interior pages don't > need to exist. Every interior chunk is always aligned to its order and > we only need to record the order. > > An overall starting offset and total length allow computing the slice > of the first/last entry. > > If the physical address is always aligned then we get 12 free bits > from the min 4k alignment and also only need to store order, not an > arbitary byte granular length. > > The win is I think we can meaningfully cover most common cases using > only 8 bytes per physical chunk. The 12 bits can be used to encode the > common orders (4k, 2M, 1G, etc) and some smart mechanism to get > another 16 bits to cover 'everything'. > > IMHO storage density here is quite important, we end up having to keep > this stuff around for a long time. > > I say this here, because I've always though bio_vec/etc are more > general than we actually need, being byte granular at every chunk. Oh, I can do you one better on the bit-packing scheme. There's a representation of every power-of-two that is naturally aligned, using just one extra bit. Let's say we have 3 bits of address space and 4 bits to represent any power of two allocation within that address space: index-0, order-0 0010 index-1, order-0 ... 1110 index-7, order-0 0001 index-0, order-1 0101 index-2, order-1 1001 index-4, order-1 1101 index-6, order-1 0011 index-0, order-2 1011 index-4, order-2 0111 index-0, order-3 has no meaning and can be used to represent an invalid range, if that's useful. The lowest clear bit decodes to the order, and (x & (x+1))/2 gets you the index. That leaves you with another 11 bits to represent something smart about partial pages. The question is whether this is the right kind of optimisation to be doing. I hear you that we want a dense format, but it's questionable whether the kind of thing you're suggesting is actually denser than this scheme. For example, if we have 1GB pages and userspace happens to have allocated pages (3, 4, 5, 6, 7, 8, 9, 10) then this can be represented as a single phyr. A power-of-two scheme would have us use four entries (3, 4-7, 8-9, 10). Using a (dma_addr, size_t) tuple makes coalescing adjacent pages very cheap. If I have to walk PTEs looking for pages which can be combined together, I end up with interesting behaviour where the length of the list shrinks and expands. Using the example above, as I walk successive PUDs, the data struct looks like this: (3) (3, 4) (3, 4-5) (3, 4-5, 6) (3, 4-7) (3, 4-7, 8) (3, 4-7, 8-9) (3, 4-7, 8-9, 10) We could end up with a situation where we stop because the array is full, even though if we kept going, it'd shrink back down below the length of the array (in this example, an array of length 2 would stop when it saw page 6, even though page 7 shrinks it back down again). > What is needed is a full scatterlist replacement, including the IOMMU > part. > > For the IOMMU I would expect the datastructure to be re-used, we start >
Re: Phyr Starter
On Tue, Jan 11, 2022 at 12:40:10PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 10.01.22 um 20:34 schrieb Matthew Wilcox:
> > TLDR: I want to introduce a new data type:
> >
> > struct phyr {
> > phys_addr_t addr;
> > size_t len;
> > };
>
> Did you look at struct dma_buf_map? [1]
Thanks. I wasn't aware of that. It doesn't seem to actually solve the
problem, in that it doesn't carry any length information. Did you mean
to point me at a different structure?
Re: Phyr Starter
On Tue, Jan 11, 2022 at 12:17:18AM -0800, John Hubbard wrote: > Zooming in on the pinning aspect for a moment: last time I attempted to > convert O_DIRECT callers from gup to pup, I recall wanting very much to > record, in each bio_vec, whether these pages were acquired via FOLL_PIN, > or some non-FOLL_PIN method. Because at the end of the IO, it is not > easy to disentangle which pages require put_page() and which require > unpin_user_page*(). > > And changing the bio_vec for *that* purpose was not really acceptable. > > But now that you're looking to change it in a big way (and with some > spare bits avaiable...oohh!), maybe I can go that direction after all. > > Or, are you looking at a design in which any phyr is implicitly FOLL_PIN'd > if it exists at all? That. I think there's still good reasons to keep a single-page (or maybe dual-page) GUP around, but no reason to mix it with ranges. > Or any other thoughts in this area are very welcome. That's there's no support for unpinning part of a range. You pin it, do the IO, unpin it. That simplifies the accounting.
Re: Phyr Starter
On Tue, Jan 11, 2022 at 11:01:42AM -0400, Jason Gunthorpe wrote:
> On Tue, Jan 11, 2022 at 04:32:56AM +0000, Matthew Wilcox wrote:
> > On Mon, Jan 10, 2022 at 08:41:26PM -0400, Jason Gunthorpe wrote:
> > > On Mon, Jan 10, 2022 at 07:34:49PM +0000, Matthew Wilcox wrote:
> > >
> > > > Finally, it may be possible to stop using scatterlist to describe the
> > > > input to the DMA-mapping operation. We may be able to get struct
> > > > scatterlist down to just dma_address and dma_length, with chaining
> > > > handled through an enclosing struct.
> > >
> > > Can you talk about this some more? IMHO one of the key properties of
> > > the scatterlist is that it can hold huge amounts of pages without
> > > having to do any kind of special allocation due to the chaining.
> > >
> > > The same will be true of the phyr idea right?
> >
> > My thinking is that we'd pass a relatively small array of phyr (maybe 16
> > entries) to get_user_phyr(). If that turned out not to be big enough,
> > then we have two options; one is to map those 16 ranges with sg and use
> > the sg chaining functionality before throwing away the phyr and calling
> > get_user_phyr() again.
>
> Then we are we using get_user_phyr() at all if we are just storing it
> in a sg?
I did consider just implementing get_user_sg() (actually 4 years ago),
but that cements the use of sg as both an input and output data structure
for DMA mapping, which I am under the impression we're trying to get
away from.
> Also 16 entries is way to small, it should be at least a whole PMD
> worth so we don't have to relock the PMD level each iteration.
>
> I would like to see a flow more like:
>
> cpu_phyr_list = get_user_phyr(uptr, 1G);
> dma_phyr_list = dma_map_phyr(device, cpu_phyr_list);
> [..]
> dma_unmap_phyr(device, dma_phyr_list);
> unpin_drity_free(cpu_phy_list);
>
> Where dma_map_phyr() can build a temporary SGL for old iommu drivers
> compatability. iommu drivers would want to implement natively, of
> course.
>
> ie no loops in drivers.
Let me just rewrite that for you ...
umem->phyrs = get_user_phyrs(addr, size, &umem->phyr_len);
umem->sgt = dma_map_phyrs(device, umem->phyrs, umem->phyr_len,
DMA_BIDIRECTIONAL, dma_attr);
...
dma_unmap_phyr(device, umem->phyrs, umem->phyr_len, umem->sgt->sgl,
umem->sgt->nents, DMA_BIDIRECTIONAL, dma_attr);
sg_free_table(umem->sgt);
free_user_phyrs(umem->phyrs, umem->phyr_len);
> > The question is whether this is the right kind of optimisation to be
> > doing. I hear you that we want a dense format, but it's questionable
> > whether the kind of thing you're suggesting is actually denser than this
> > scheme. For example, if we have 1GB pages and userspace happens to have
> > allocated pages (3, 4, 5, 6, 7, 8, 9, 10) then this can be represented
> > as a single phyr. A power-of-two scheme would have us use four entries
> > (3, 4-7, 8-9, 10).
>
> That is not quite what I had in mind..
>
> struct phyr_list {
>unsigned int first_page_offset_bytes;
>size_t total_length_bytes;
>phys_addr_t min_alignment;
>struct packed_phyr *list_of_pages;
> };
>
> Where each 'packed_phyr' is an aligned page of some kind. The packing
> has to be able to represent any number of pfns, so we have four major
> cases:
> - 4k pfns (use 8 bytes)
> - Natural order pfn (use 8 bytes)
> - 4k aligned pfns, arbitary number (use 12 bytes)
> - <4k aligned, arbitary length (use 16 bytes?)
>
> In all cases the interior pages are fully used, only the first and
> last page is sliced based on the two parameters in the phyr_list.
This kind of representation works for a virtually contiguous range.
Unfortunately, that's not sufficient for some bio users (as I discovered
after getting a representation like this enshrined in the NVMe spec as
the PRP List).
> The last case is, perhaps, a possible route to completely replace
> scatterlist. Few places need true byte granularity for interior pages,
> so we can invent some coding to say 'this is 8 byte aligned, and n
> bytes long' that only fits < 4k or something. Exceptional cases can
> then still work. I'm not sure what block needs here - is it just 512?
Replacing scatterlist is not my goal. That seems like a lot more work
for little gain. I just want to delete page_link, offset and length
from struct scatterlist. Given the above sequence of calls, we're going
to get sg lists that aren't chained. They may have to be vmalloced,
but th
Re: Phyr Starter
On Tue, Jan 11, 2022 at 04:21:59PM -0400, Jason Gunthorpe wrote: > On Tue, Jan 11, 2022 at 06:33:57PM +0000, Matthew Wilcox wrote: > > > > Then we are we using get_user_phyr() at all if we are just storing it > > > in a sg? > > > > I did consider just implementing get_user_sg() (actually 4 years ago), > > but that cements the use of sg as both an input and output data structure > > for DMA mapping, which I am under the impression we're trying to get > > away from. > > I know every time I talked about a get_user_sg() Christoph is against > it and we need to stop using scatter list... > > > > Also 16 entries is way to small, it should be at least a whole PMD > > > worth so we don't have to relock the PMD level each iteration. > > > > > > I would like to see a flow more like: > > > > > > cpu_phyr_list = get_user_phyr(uptr, 1G); > > > dma_phyr_list = dma_map_phyr(device, cpu_phyr_list); > > > [..] > > > dma_unmap_phyr(device, dma_phyr_list); > > > unpin_drity_free(cpu_phy_list); > > > > > > Where dma_map_phyr() can build a temporary SGL for old iommu drivers > > > compatability. iommu drivers would want to implement natively, of > > > course. > > > > > > ie no loops in drivers. > > > > Let me just rewrite that for you ... > > > > umem->phyrs = get_user_phyrs(addr, size, &umem->phyr_len); > > umem->sgt = dma_map_phyrs(device, umem->phyrs, umem->phyr_len, > > DMA_BIDIRECTIONAL, dma_attr); > > ... > > dma_unmap_phyr(device, umem->phyrs, umem->phyr_len, umem->sgt->sgl, > > umem->sgt->nents, DMA_BIDIRECTIONAL, dma_attr); > > sg_free_table(umem->sgt); > > free_user_phyrs(umem->phyrs, umem->phyr_len); > > Why? As above we want to get rid of the sgl, so you are telling me to > adopt phyrs I need to increase the memory consumption by a hefty > amount to store the phyrs and still keep the sgt now? Why? > > I don't need the sgt at all. I just need another list of physical > addresses for DMA. I see no issue with a phsr_list storing either CPU > Physical Address or DMA Physical Addresses, same data structure. There's a difference between a phys_addr_t and a dma_addr_t. They can even be different sizes; some architectures use a 32-bit dma_addr_t and a 64-bit phys_addr_t or vice-versa. phyr cannot store DMA addresses. > In the fairly important passthrough DMA case the CPU list and DMA list > are identical, so we don't even need to do anything. > > In the typical iommu case my dma map's phyrs is only one entry. That becomes a very simple sg table then. > As an example coding - Use the first 8 bytes to encode this: > > 51:0 - Physical address / 4k (ie pfn) > 56:52 - Order (simple, your order encoding can do better) > 61:57 - Unused > 63:62 - Mode, one of: > 00 = natural order pfn (8 bytes) > 01 = order aligned with length (12 bytes) > 10 = arbitary (12 bytes) > > Then the optional 4 bytes are used as: > > Mode 01 (Up to 2^48 bytes of memory on a 4k alignment) > 31:0 - # of order pages > > Mode 10 (Up to 2^25 bytes of memory on a 1 byte alignment) > 11:0 - starting byte offset in the 4k > 31:12 - 20 bits, plus the 5 bit order from the first 8 bytes: > length in bytes Honestly, this looks awful to operate on. Mandatory 8-bytes per entry with an optional 4 byte extension? > > > The last case is, perhaps, a possible route to completely replace > > > scatterlist. Few places need true byte granularity for interior pages, > > > so we can invent some coding to say 'this is 8 byte aligned, and n > > > bytes long' that only fits < 4k or something. Exceptional cases can > > > then still work. I'm not sure what block needs here - is it just 512? > > > > Replacing scatterlist is not my goal. That seems like a lot more work > > for little gain. > > Well, I'm not comfortable with the idea above where RDMA would have to > take a memory penalty to use the new interface. To avoid that memory > penalty we need to get rid of scatterlist entirely. > > If we do the 16 byte struct from the first email then a umem for MRs > will increase in memory consumption by 160% compared today's 24 > bytes/page. I think the HPC workloads will veto this. Huh? We do 16 bytes per physically contiguous range. Then, if your HPC workloads use an IOMMU that can map a virtually contiguous range into a single sg entry, it uses 24 bytes for the entire mapping. It s
Re: Phyr Starter
On Tue, Jan 11, 2022 at 06:53:06PM -0400, Jason Gunthorpe wrote: > IOMMU is not common in those cases, it is slow. > > So you end up with 16 bytes per entry then another 24 bytes in the > entirely redundant scatter list. That is now 40 bytes/page for typical > HPC case, and I can't see that being OK. Ah, I didn't realise what case you wanted to optimise for. So, how about this ... Since you want to get to the same destination as I do (a 16-byte-per-entry dma_addr+dma_len struct), but need to get there sooner than "make all sg users stop using it wrongly", let's introduce a (hopefully temporary) "struct dma_range". But let's go further than that (which only brings us to 32 bytes per range). For the systems you care about which use an identity mapping, and have sizeof(dma_addr_t) == sizeof(phys_addr_t), we can simply point the dma_range pointer to the same memory as the phyr. We just have to not free it too early. That gets us down to 16 bytes per range, a saving of 33%.
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote:
> Drop the pgtable_t variable from all implementation for pte_fn_t as none of
> them use it. apply_to_pte_range() should stop computing it as well. Should
> help us save some cycles.
You didn't add Martin Schwidefsky for some reason. He introduced
it originally for s390 for sub-page page tables back in 2008 (commit
2f569afd9c). I think he should confirm that he no longer needs it.
> ---
> - Boot tested on arm64 and x86 platforms.
> - Build tested on multiple platforms with their defconfig
>
> arch/arm/kernel/efi.c | 3 +--
> arch/arm/mm/dma-mapping.c | 3 +--
> arch/arm/mm/pageattr.c | 3 +--
> arch/arm64/kernel/efi.c| 3 +--
> arch/arm64/mm/pageattr.c | 3 +--
> arch/x86/xen/mmu_pv.c | 3 +--
> drivers/gpu/drm/i915/i915_mm.c | 3 +--
> drivers/xen/gntdev.c | 6 ++
> drivers/xen/privcmd.c | 6 ++
> drivers/xen/xlate_mmu.c| 3 +--
> include/linux/mm.h | 3 +--
> mm/memory.c| 5 +
> mm/vmalloc.c | 2 +-
> 13 files changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> index 9f43ba012d10..b1f142a01f2f 100644
> --- a/arch/arm/kernel/efi.c
> +++ b/arch/arm/kernel/efi.c
> @@ -11,8 +11,7 @@
> #include
> #include
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 43f46aa7ef33..739286511a18 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
> }
> }
>
> -static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long addr,
> - void *data)
> +static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
> {
> struct page *page = virt_to_page(addr);
> pgprot_t prot = *(pgprot_t *)data;
> diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> index 1403cb4a0c3d..c8b500940e1f 100644
> --- a/arch/arm/mm/pageattr.c
> +++ b/arch/arm/mm/pageattr.c
> @@ -22,8 +22,7 @@ struct page_change_data {
> pgprot_t clear_mask;
> };
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 4f9acb5fbe97..230cff073a08 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
> efi_memory_desc_t *md)
> return 0;
> }
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 6cd645edcf35..0be077628b21 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -27,8 +27,7 @@ struct page_change_data {
>
> bool rodata_full __ro_after_init =
> IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index a21e1734fc1f..308a6195fd26 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2702,8 +2702,7 @@ struct remap_data {
> struct mmu_update *mmu_update;
> };
>
> -static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void *data)
> {
> struct remap_data *rmd = data;
> pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot));
> diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
> index e4935dd1fd37..c23bb29e6d3e 100644
> --- a/drivers/gpu/drm/i915/i915_mm.c
> +++ b/drivers/gpu/drm/i915/i915_mm.c
> @@ -35,8 +35,7 @@ struct remap_pfn {
> pgprot_t prot;
> };
>
> -static int remap_pfn(pte_t *pte, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
> {
> str
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, May 02, 2019 at 04:14:57PM +0200, Martin Schwidefsky wrote: > On Thu, 2 May 2019 06:46:23 -0700 > Matthew Wilcox wrote: > > > On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote: > > > Drop the pgtable_t variable from all implementation for pte_fn_t as none > > > of > > > them use it. apply_to_pte_range() should stop computing it as well. Should > > > help us save some cycles. > > > > You didn't add Martin Schwidefsky for some reason. He introduced > > it originally for s390 for sub-page page tables back in 2008 (commit > > 2f569afd9c). I think he should confirm that he no longer needs it. > > With its 2K pte tables s390 can not deal with a (struct page *) as a reference > to a page table. But if there are no user of the apply_to_page_range() API > left which actually make use of the token argument we can safely drop it. Interestingly, I don't think there ever was a user which used that argument. Looking at your 2f56 patch, you only converted one function (presumably there was only one caller of apply_to_page_range() at the time), and it didn't u se any of the arguments. Xen was the initial user, and the two other functions they added also didn't use that argument. Looking at a quick sample of users added since, none of them appear to have ever used that argument. So removing it seems best. Acked-by: Matthew Wilcox ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches
On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:
> Can't have:
>
> switch (i) {
> int j;
> case 0:
> /* ... */
> }
>
> because it can't be turned into:
>
> switch (i) {
> int j = 0; /* not valid C */
> case 0:
> /* ... */
> }
>
> but can have e.g.:
>
> switch (i) {
> case 0:
> {
> int j = 0;
> /* ... */
> }
> }
>
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.
Another nesting level would be bad, but I think this is OK:
switch (i) {
case 0: {
int j = 0;
/* ... */
}
case 1: {
void *p = q;
/* ... */
}
}
I can imagine Kees' patch might have a bad effect on stack consumption,
unless GCC can be relied on to be smart enough to notice the
non-overlapping liveness of the vriables and use the same stack slots
for both.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 1/9] mm: Introduce new vm_insert_range and vm_insert_range_buggy API
On Thu, Feb 07, 2019 at 09:19:47PM +0530, Souptick Joarder wrote: > Just thought to take opinion for documentation before placing it in v3. > Does it looks fine ? > > +/** > + * __vm_insert_range - insert range of kernel pages into user vma > + * @vma: user vma to map to > + * @pages: pointer to array of source kernel pages > + * @num: number of pages in page array > + * @offset: user's requested vm_pgoff > + * > + * This allow drivers to insert range of kernel pages into a user vma. > + * > + * Return: 0 on success and error code otherwise. > + */ > +static int __vm_insert_range(struct vm_area_struct *vma, struct page **pages, > + unsigned long num, unsigned long offset) For static functions, I prefer to leave off the second '*', ie make it formatted like a docbook comment, but not be processed like a docbook comment. That avoids cluttering the html with descriptions of internal functions that people can't actually call. > +/** > + * vm_insert_range - insert range of kernel pages starts with non zero offset > + * @vma: user vma to map to > + * @pages: pointer to array of source kernel pages > + * @num: number of pages in page array > + * > + * Maps an object consisting of `num' `pages', catering for the user's Rather than using `num', you should use @num. > + * requested vm_pgoff > + * > + * If we fail to insert any page into the vma, the function will return > + * immediately leaving any previously inserted pages present. Callers > + * from the mmap handler may immediately return the error as their caller > + * will destroy the vma, removing any successfully inserted pages. Other > + * callers should make their own arrangements for calling unmap_region(). > + * > + * Context: Process context. Called by mmap handlers. > + * Return: 0 on success and error code otherwise. > + */ > +int vm_insert_range(struct vm_area_struct *vma, struct page **pages, > + unsigned long num) > > > +/** > + * vm_insert_range_buggy - insert range of kernel pages starts with zero > offset > + * @vma: user vma to map to > + * @pages: pointer to array of source kernel pages > + * @num: number of pages in page array > + * > + * Similar to vm_insert_range(), except that it explicitly sets @vm_pgoff to But vm_pgoff isn't a parameter, so it's misleading to format it as such. > + * 0. This function is intended for the drivers that did not consider > + * @vm_pgoff. > + * > + * Context: Process context. Called by mmap handlers. > + * Return: 0 on success and error code otherwise. > + */ > +int vm_insert_range_buggy(struct vm_area_struct *vma, struct page **pages, > + unsigned long num) I don't think we should call it 'buggy'. 'zero' would make more sense as a suffix. Given how this interface has evolved, I'm no longer sure than 'vm_insert_range' makes sense as the name for it. Is it perhaps 'vm_map_object' or 'vm_map_pages'? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Build etnaviv on non-ARM architectures
I wanted to test-compile etnaviv on x86 after making a tree-wide change to it. Unfortunately, Kconfig has a bad dependency, so I couldn't. Signed-off-by: Matthew Wilcox diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig index 041a77e400d4..342591a1084e 100644 --- a/drivers/gpu/drm/etnaviv/Kconfig +++ b/drivers/gpu/drm/etnaviv/Kconfig @@ -2,7 +2,7 @@ config DRM_ETNAVIV tristate "ETNAVIV (DRM support for Vivante GPU IP cores)" depends on DRM - depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST) + depends on ARCH_MXC || ARCH_DOVE || ARM || COMPILE_TEST depends on MMU select SHMEM select SYNC_FILE ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] etnaviv mailing list is moderated
Signed-off-by: Matthew Wilcox diff --git a/MAINTAINERS b/MAINTAINERS index 32d76a90..44888eb121d8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5141,7 +5141,7 @@ DRM DRIVERS FOR VIVANTE GPU IP M: Lucas Stach R: Russell King R: Christian Gmeiner -L: etna...@lists.freedesktop.org +L: etna...@lists.freedesktop.org (moderated for non-subscribers) L: dri-devel@lists.freedesktop.org S: Maintained F: drivers/gpu/drm/etnaviv/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/34] drm: Remove linked lists for lessees
These are already tracked in the XArray so we do not need to also keep
a doubly-linked list.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_auth.c | 4 +--
drivers/gpu/drm/drm_lease.c | 58 ++---
include/drm/drm_auth.h | 4 +--
3 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 28767f55b30b..1813507f9b9c 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -110,10 +110,8 @@ struct drm_master *drm_master_create(struct drm_device
*dev)
/* initialize the tree of output resource lessees */
master->lessor = NULL;
master->lessee_id = 0;
- INIT_LIST_HEAD(&master->lessees);
- INIT_LIST_HEAD(&master->lessee_list);
idr_init(&master->leases);
- xa_init_flags(&master->lessee_xa, XA_FLAGS_ALLOC1);
+ xa_init_flags(&master->lessees, XA_FLAGS_ALLOC1);
return master;
}
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index c02587443b61..47830f9ec616 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -20,9 +20,6 @@
#include
#include
-#define drm_for_each_lessee(lessee, lessor) \
- list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
-
static uint64_t drm_lease_idr_object;
/**
@@ -54,7 +51,7 @@ static struct drm_master*
_drm_find_lessee(struct drm_master *master, int lessee_id)
{
lockdep_assert_held(&master->dev->mode_config.idr_mutex);
- return xa_load(&drm_lease_owner(master)->lessee_xa, lessee_id);
+ return xa_load(&drm_lease_owner(master)->lessees, lessee_id);
}
/**
@@ -90,9 +87,10 @@ static int _drm_lease_held_master(struct drm_master *master,
int id)
static bool _drm_has_leased(struct drm_master *master, int id)
{
struct drm_master *lessee;
+ unsigned long index;
lockdep_assert_held(&master->dev->mode_config.idr_mutex);
- drm_for_each_lessee(lessee, master)
+ xa_for_each(&master->lessees, index, lessee)
if (_drm_lease_held_master(lessee, id))
return true;
return false;
@@ -231,13 +229,12 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
}
/* Insert the new lessee into the tree */
- error = xa_alloc(&drm_lease_owner(lessor)->lessee_xa,
- &lessee->lessee_id, lessee, xa_limit_32b, GFP_KERNEL);
+ error = xa_alloc(&drm_lease_owner(lessor)->lessees, &lessee->lessee_id,
+ lessee, xa_limit_32b, GFP_KERNEL);
if (error < 0)
goto out_lessee;
lessee->lessor = drm_master_get(lessor);
- list_add_tail(&lessee->lessee_list, &lessor->lessees);
/* Move the leases over */
lessee->leases = *leases;
@@ -271,20 +268,13 @@ void drm_lease_destroy(struct drm_master *master)
DRM_DEBUG_LEASE("drm_lease_destroy %d\n", master->lessee_id);
- /* This master is referenced by all lessees, hence it cannot be
destroyed
-* until all of them have been
-*/
- WARN_ON(!list_empty(&master->lessees));
+ WARN_ON(!xa_empty(&master->lessees));
/* Remove this master from the lessee array in the owner */
if (master->lessee_id != 0) {
DRM_DEBUG_LEASE("remove master %d from device list of
lessees\n", master->lessee_id);
- xa_erase(&drm_lease_owner(master)->lessee_xa,
master->lessee_id);
+ xa_erase(&drm_lease_owner(master)->lessees, master->lessee_id);
}
-
- /* Remove this master from any lessee list it may be on */
- list_del(&master->lessee_list);
-
mutex_unlock(&dev->mode_config.idr_mutex);
if (master->lessor) {
@@ -313,27 +303,34 @@ static void _drm_lease_revoke(struct drm_master *top)
* the tree is fully connected, we can do this without recursing
*/
for (;;) {
+ struct drm_master *tmp;
+ unsigned long index = 0;
+
DRM_DEBUG_LEASE("revoke leases for %p %d\n", master,
master->lessee_id);
/* Evacuate the lease */
idr_for_each_entry(&master->leases, entry, object)
idr_remove(&master->leases, object);
- /* Depth-first list walk */
+ /* Depth-first tree walk */
+ tmp = xa_find(&master->lessees, &index, ULONG_MAX, XA_PRESENT);
/* Down */
- if (!list_empty(&master->lessees)) {
- master = list_first_entry(&master->lessees, struct
drm_master, lessee_list);
-
[PATCH 25/34] drm/i915: Convert vgpus_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/gvt/display.c | 5 +++--
drivers/gpu/drm/i915/gvt/gvt.c | 4 +---
drivers/gpu/drm/i915/gvt/gvt.h | 5 +++--
drivers/gpu/drm/i915/gvt/kvmgt.c| 2 +-
drivers/gpu/drm/i915/gvt/sched_policy.c | 2 +-
drivers/gpu/drm/i915/gvt/vgpu.c | 20 ++--
6 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/display.c
b/drivers/gpu/drm/i915/gvt/display.c
index df1e14145747..e0b25efd9fee 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -359,7 +359,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
{
struct intel_gvt_irq *irq = &gvt->irq;
struct intel_vgpu *vgpu;
- int pipe, id;
+ unsigned long id;
+ int pipe;
int found = false;
mutex_lock(&gvt->lock);
@@ -434,7 +435,7 @@ static void emulate_vblank(struct intel_vgpu *vgpu)
void intel_gvt_emulate_vblank(struct intel_gvt *gvt)
{
struct intel_vgpu *vgpu;
- int id;
+ unsigned long id;
mutex_lock(&gvt->lock);
for_each_active_vgpu(gvt, vgpu, id)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 733a2a0d0c30..ef6c62ebc2be 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -329,7 +329,6 @@ void intel_gvt_clean_device(struct drm_i915_private
*dev_priv)
intel_gvt_clean_irq(gvt);
intel_gvt_free_firmware(gvt);
intel_gvt_clean_mmio_info(gvt);
- idr_destroy(&gvt->vgpu_idr);
kfree(dev_priv->gvt);
dev_priv->gvt = NULL;
@@ -368,7 +367,7 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv)
gvt_dbg_core("init gvt device\n");
- idr_init(&gvt->vgpu_idr);
+ xa_init_flags(&gvt->vgpus, XA_FLAGS_ALLOC1);
spin_lock_init(&gvt->scheduler.mmio_context_lock);
mutex_init(&gvt->lock);
mutex_init(&gvt->sched_lock);
@@ -462,7 +461,6 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv)
out_clean_mmio_info:
intel_gvt_clean_mmio_info(gvt);
out_clean_idr:
- idr_destroy(&gvt->vgpu_idr);
kfree(gvt);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index ffb181a086be..843d199dc82a 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -314,7 +314,7 @@ struct intel_gvt {
struct mutex sched_lock;
struct drm_i915_private *dev_priv;
- struct idr vgpu_idr;/* vGPU IDR pool */
+ struct xarray vgpus;/* vGPU IDR pool */
struct intel_gvt_device_info device_info;
struct intel_gvt_gm gm;
@@ -458,8 +458,9 @@ void intel_vgpu_write_fence(struct intel_vgpu *vgpu,
#define vgpu_sreg(vgpu, offset) \
(*(u32 *)(vgpu->mmio.sreg + (offset)))
+/* Can this use an XArray mark instead? */
#define for_each_active_vgpu(gvt, vgpu, id) \
- idr_for_each_entry((&(gvt)->vgpu_idr), (vgpu), (id)) \
+ xa_for_each((&(gvt)->vgpus), (id), (vgpu)) \
for_each_if(vgpu->active)
static inline void intel_vgpu_write_pci_bar(struct intel_vgpu *vgpu,
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index dd3dfd00f4e6..7f761879c14a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1587,7 +1587,7 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu,
struct kvm *kvm)
{
struct intel_vgpu *itr;
struct kvmgt_guest_info *info;
- int id;
+ unsigned long id;
bool ret = false;
mutex_lock(&vgpu->gvt->lock);
diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c
b/drivers/gpu/drm/i915/gvt/sched_policy.c
index c32e7d5e8629..f0223e0a1a23 100644
--- a/drivers/gpu/drm/i915/gvt/sched_policy.c
+++ b/drivers/gpu/drm/i915/gvt/sched_policy.c
@@ -334,7 +334,7 @@ static void tbs_sched_clean_vgpu(struct intel_vgpu *vgpu)
vgpu->sched_data = NULL;
/* this vgpu id has been removed */
- if (idr_is_empty(&gvt->vgpu_idr))
+ if (xa_empty(&gvt->vgpus))
hrtimer_cancel(&sched_data->timer);
}
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index c1db9a6a1281..5dfdfa20331f 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -290,8 +290,8 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
mutex_unlock(&vgpu->vgpu_lock);
mutex_lock(&gvt->lock);
- idr_remove(&gvt->vgpu_idr, vgpu->id);
- if (idr_is_empty(&gvt->vgpu_idr))
+ xa_erase(&gvt->vgpus, vgpu->id);
+ if (xa_empty(&gvt->vgpus))
intel_gvt_clean_irq(gvt);
intel_gvt_update_vgpu_typ
[PATCH 09/34] drm: Convert ctx_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_context.c | 42 +++
include/drm/drm_device.h | 2 +-
2 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index 506663c69b0a..b498e311cf57 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -48,8 +48,7 @@ struct drm_ctx_list {
* \param ctx_handle context handle.
*
* Clears the bit specified by \p ctx_handle in drm_device::ctx_bitmap and the
entry
- * in drm_device::ctx_idr, while holding the drm_device::struct_mutex
- * lock.
+ * in drm_device::ctxts.
*/
void drm_legacy_ctxbitmap_free(struct drm_device * dev, int ctx_handle)
{
@@ -57,9 +56,7 @@ void drm_legacy_ctxbitmap_free(struct drm_device * dev, int
ctx_handle)
!drm_core_check_feature(dev, DRIVER_LEGACY))
return;
- mutex_lock(&dev->struct_mutex);
- idr_remove(&dev->ctx_idr, ctx_handle);
- mutex_unlock(&dev->struct_mutex);
+ xa_erase(&dev->ctxts, ctx_handle);
}
/**
@@ -68,18 +65,17 @@ void drm_legacy_ctxbitmap_free(struct drm_device * dev, int
ctx_handle)
* \param dev DRM device.
* \return (non-negative) context handle on success or a negative number on
failure.
*
- * Allocate a new idr from drm_device::ctx_idr while holding the
- * drm_device::struct_mutex lock.
+ * Allocate a new id from drm_device::ctxts.
*/
static int drm_legacy_ctxbitmap_next(struct drm_device * dev)
{
- int ret;
+ int ret, id;
- mutex_lock(&dev->struct_mutex);
- ret = idr_alloc(&dev->ctx_idr, NULL, DRM_RESERVED_CONTEXTS, 0,
- GFP_KERNEL);
- mutex_unlock(&dev->struct_mutex);
- return ret;
+ ret = xa_alloc(&dev->ctxts, &id, NULL,
+ XA_LIMIT(DRM_RESERVED_CONTEXTS, INT_MAX), GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ return id;
}
/**
@@ -87,7 +83,7 @@ static int drm_legacy_ctxbitmap_next(struct drm_device * dev)
*
* \param dev DRM device.
*
- * Initialise the drm_device::ctx_idr
+ * Initialise the drm_device::ctxts
*/
void drm_legacy_ctxbitmap_init(struct drm_device * dev)
{
@@ -95,7 +91,7 @@ void drm_legacy_ctxbitmap_init(struct drm_device * dev)
!drm_core_check_feature(dev, DRIVER_LEGACY))
return;
- idr_init(&dev->ctx_idr);
+ xa_init_flags(&dev->ctxts, XA_FLAGS_ALLOC);
}
/**
@@ -103,8 +99,8 @@ void drm_legacy_ctxbitmap_init(struct drm_device * dev)
*
* \param dev DRM device.
*
- * Free all idr members using drm_ctx_sarea_free helper function
- * while holding the drm_device::struct_mutex lock.
+ * Free all memory used by the ctxts data structure. Does not free the
+ * pointers in that data structures.
*/
void drm_legacy_ctxbitmap_cleanup(struct drm_device * dev)
{
@@ -112,9 +108,7 @@ void drm_legacy_ctxbitmap_cleanup(struct drm_device * dev)
!drm_core_check_feature(dev, DRIVER_LEGACY))
return;
- mutex_lock(&dev->struct_mutex);
- idr_destroy(&dev->ctx_idr);
- mutex_unlock(&dev->struct_mutex);
+ xa_destroy(&dev->ctxts);
}
/**
@@ -166,7 +160,7 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev,
struct drm_file *file)
* \param arg user argument pointing to a drm_ctx_priv_map structure.
* \return zero on success or a negative number on failure.
*
- * Gets the map from drm_device::ctx_idr with the handle specified and
+ * Gets the map from drm_device::ctxts with the handle specified and
* returns its handle.
*/
int drm_legacy_getsareactx(struct drm_device *dev, void *data,
@@ -182,7 +176,7 @@ int drm_legacy_getsareactx(struct drm_device *dev, void
*data,
mutex_lock(&dev->struct_mutex);
- map = idr_find(&dev->ctx_idr, request->ctx_id);
+ map = xa_load(&dev->ctxts, request->ctx_id);
if (!map) {
mutex_unlock(&dev->struct_mutex);
return -EINVAL;
@@ -215,7 +209,7 @@ int drm_legacy_getsareactx(struct drm_device *dev, void
*data,
* \return zero on success or a negative number on failure.
*
* Searches the mapping specified in \p arg and update the entry in
- * drm_device::ctx_idr with it.
+ * drm_device::ctxts with it.
*/
int drm_legacy_setsareactx(struct drm_device *dev, void *data,
struct drm_file *file_priv)
@@ -243,7 +237,7 @@ int drm_legacy_setsareactx(struct drm_device *dev, void
*data,
if (!map)
goto bad;
- if (IS_ERR(idr_replace(&dev->ctx_idr, map, request->ctx_id)))
+ if (xa_is_err(xa_store(&dev->ctxts, request->ctx_id, map, GFP_KERNEL)))
goto bad;
mutex_unlock(&dev->struct_mutex);
diff --git a/include/drm/drm_device.h
[PATCH 21/34] drm/i915: Convert get_page to XArray
This initially seemed like an ideal use-case for the store_range
functionality, but there's no easy way to determine where we were
relative to the base of the entry. So this is a straightforward
conversion which doesn't even touch the locking.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/i915_gem.c | 38 ++---
drivers/gpu/drm/i915/i915_gem_context.h | 2 +-
drivers/gpu/drm/i915/i915_gem_object.h | 4 +--
3 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0cdccc886587..f8c36c2d32c2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2428,13 +2428,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object
*obj,
static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
{
- struct radix_tree_iter iter;
- void __rcu **slot;
-
- rcu_read_lock();
- radix_tree_for_each_slot(slot, &obj->mm.get_page.radix, &iter, 0)
- radix_tree_delete(&obj->mm.get_page.radix, iter.index);
- rcu_read_unlock();
+ xa_destroy(&obj->mm.get_page.xa);
}
static struct sg_table *
@@ -4716,7 +4710,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
init_request_active(&obj->frontbuffer_write, frontbuffer_retire);
obj->mm.madv = I915_MADV_WILLNEED;
- INIT_RADIX_TREE(&obj->mm.get_page.radix, GFP_KERNEL | __GFP_NOWARN);
+ xa_init(&obj->mm.get_page.xa);
mutex_init(&obj->mm.get_page.lock);
i915_gem_info_add_obj(to_i915(obj->base.dev), obj->base.size);
@@ -6073,8 +6067,8 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
GEM_BUG_ON(n >= obj->base.size >> PAGE_SHIFT);
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
- /* As we iterate forward through the sg, we record each entry in a
-* radixtree for quick repeated (backwards) lookups. If we have seen
+ /* As we iterate forward through the sg, we record each entry in an
+* xarray for quick repeated (backwards) lookups. If we have seen
* this index previously, we will have an entry for it.
*
* Initial lookup is O(N), but this is amortized to O(1) for
@@ -6089,7 +6083,7 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
/* We prefer to reuse the last sg so that repeated lookup of this
* (or the subsequent) sg are fast - comparing against the last
-* sg is faster than going through the radixtree.
+* sg is faster than going through the xarray.
*/
sg = iter->sg_pos;
@@ -6099,24 +6093,22 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
while (idx + count <= n) {
void *entry;
unsigned long i;
- int ret;
/* If we cannot allocate and insert this entry, or the
* individual pages from this range, cancel updating the
* sg_idx so that on this lookup we are forced to linearly
* scan onwards, but on future lookups we will try the
-* insertion again (in which case we need to be careful of
-* the error return reporting that we have already inserted
-* this index).
+* insertion again.
*/
- ret = radix_tree_insert(&iter->radix, idx, sg);
- if (ret && ret != -EEXIST)
+ if (xa_store(&iter->xa, idx, sg, GFP_KERNEL | __GFP_NOWARN)
+ == XA_ERROR(-ENOMEM))
goto scan;
entry = xa_mk_value(idx);
for (i = 1; i < count; i++) {
- ret = radix_tree_insert(&iter->radix, idx + i, entry);
- if (ret && ret != -EEXIST)
+ if (xa_store(&iter->xa, idx + i, entry,
+ GFP_KERNEL | __GFP_NOWARN)
+ == XA_ERROR(-ENOMEM))
goto scan;
}
@@ -6134,7 +6126,7 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
if (unlikely(n < idx)) /* insertion completed by another thread */
goto lookup;
- /* In case we failed to insert the entry into the radixtree, we need
+ /* In case we failed to insert the entry into the xarray, we need
* to look beyond the current sg.
*/
while (idx + count <= n) {
@@ -6149,11 +6141,11 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
lookup:
rcu_read_lock();
- sg = radix_tree_lookup(&iter->radix, n);
+ sg = xa_load(&iter->xa, n);
GEM_BUG_ON(!sg);
/* If this index is in the
[PATCH 05/34] drm: Convert syncobj_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_syncobj.c | 64 +++
include/drm/drm_file.h| 6 ++--
2 files changed, 22 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index db30a0e89db8..3b63b1eeec80 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -69,14 +69,12 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file
*file_private,
{
struct drm_syncobj *syncobj;
- spin_lock(&file_private->syncobj_table_lock);
-
- /* Check if we currently have a reference on the object */
- syncobj = idr_find(&file_private->syncobj_idr, handle);
+ /* Get a reference on the object */
+ xa_lock(&file_private->syncobjs);
+ syncobj = xa_load(&file_private->syncobjs, handle);
if (syncobj)
drm_syncobj_get(syncobj);
-
- spin_unlock(&file_private->syncobj_table_lock);
+ xa_unlock(&file_private->syncobjs);
return syncobj;
}
@@ -288,23 +286,16 @@ int drm_syncobj_get_handle(struct drm_file *file_private,
{
int ret;
- /* take a reference to put in the idr */
+ /* take a reference to put in the XArray */
drm_syncobj_get(syncobj);
- idr_preload(GFP_KERNEL);
- spin_lock(&file_private->syncobj_table_lock);
- ret = idr_alloc(&file_private->syncobj_idr, syncobj, 1, 0, GFP_NOWAIT);
- spin_unlock(&file_private->syncobj_table_lock);
+ ret = xa_alloc(&file_private->syncobjs, handle, syncobj, xa_limit_31b,
+ GFP_KERNEL);
- idr_preload_end();
-
- if (ret < 0) {
+ if (ret < 0)
drm_syncobj_put(syncobj);
- return ret;
- }
- *handle = ret;
- return 0;
+ return ret;
}
EXPORT_SYMBOL(drm_syncobj_get_handle);
@@ -328,9 +319,7 @@ static int drm_syncobj_destroy(struct drm_file
*file_private,
{
struct drm_syncobj *syncobj;
- spin_lock(&file_private->syncobj_table_lock);
- syncobj = idr_remove(&file_private->syncobj_idr, handle);
- spin_unlock(&file_private->syncobj_table_lock);
+ syncobj = xa_erase(&file_private->syncobjs, handle);
if (!syncobj)
return -EINVAL;
@@ -419,16 +408,10 @@ static int drm_syncobj_fd_to_handle(struct drm_file
*file_private,
syncobj = file->private_data;
drm_syncobj_get(syncobj);
- idr_preload(GFP_KERNEL);
- spin_lock(&file_private->syncobj_table_lock);
- ret = idr_alloc(&file_private->syncobj_idr, syncobj, 1, 0, GFP_NOWAIT);
- spin_unlock(&file_private->syncobj_table_lock);
- idr_preload_end();
+ ret = xa_alloc(&file_private->syncobjs, handle, syncobj, xa_limit_31b,
+ GFP_KERNEL);
- if (ret > 0) {
- *handle = ret;
- ret = 0;
- } else
+ if (ret < 0)
drm_syncobj_put(syncobj);
fput(file);
@@ -498,17 +481,7 @@ static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
void
drm_syncobj_open(struct drm_file *file_private)
{
- idr_init_base(&file_private->syncobj_idr, 1);
- spin_lock_init(&file_private->syncobj_table_lock);
-}
-
-static int
-drm_syncobj_release_handle(int id, void *ptr, void *data)
-{
- struct drm_syncobj *syncobj = ptr;
-
- drm_syncobj_put(syncobj);
- return 0;
+ xa_init_flags(&file_private->syncobjs, XA_FLAGS_ALLOC1);
}
/**
@@ -522,9 +495,12 @@ drm_syncobj_release_handle(int id, void *ptr, void *data)
void
drm_syncobj_release(struct drm_file *file_private)
{
- idr_for_each(&file_private->syncobj_idr,
-&drm_syncobj_release_handle, file_private);
- idr_destroy(&file_private->syncobj_idr);
+ struct drm_syncobj *syncobj;
+ unsigned long index;
+
+ xa_for_each(&file_private->syncobjs, index, syncobj)
+ drm_syncobj_put(syncobj);
+ xa_destroy(&file_private->syncobjs);
}
int
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 685f3cd9d071..c10dca3c4cda 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -247,10 +247,8 @@ struct drm_file {
*/
struct xarray objects;
- /** @syncobj_idr: Mapping of sync object handles to object pointers. */
- struct idr syncobj_idr;
- /** @syncobj_table_lock: Protects @syncobj_idr. */
- spinlock_t syncobj_table_lock;
+ /** @syncobjs: Mapping of sync object handles to object pointers. */
+ struct xarray syncobjs;
/** @filp: Pointer to the core file structure. */
struct file *filp;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 20/34] drm/i915: Convert page_track_tree to XArray
No locking changes.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/gvt/gvt.h| 2 +-
drivers/gpu/drm/i915/gvt/page_track.c | 6 +++---
drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b4ab1dad0143..e5bf20dcdd7d 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -198,7 +198,7 @@ struct intel_vgpu {
struct intel_vgpu_opregion opregion;
struct intel_vgpu_display display;
struct intel_vgpu_submission submission;
- struct radix_tree_root page_track_tree;
+ struct xarray page_track;
u32 hws_pga[I915_NUM_ENGINES];
struct dentry *debugfs;
diff --git a/drivers/gpu/drm/i915/gvt/page_track.c
b/drivers/gpu/drm/i915/gvt/page_track.c
index 84856022528e..8e8b5935f344 100644
--- a/drivers/gpu/drm/i915/gvt/page_track.c
+++ b/drivers/gpu/drm/i915/gvt/page_track.c
@@ -34,7 +34,7 @@
struct intel_vgpu_page_track *intel_vgpu_find_page_track(
struct intel_vgpu *vgpu, unsigned long gfn)
{
- return radix_tree_lookup(&vgpu->page_track_tree, gfn);
+ return xa_load(&vgpu->page_track, gfn);
}
/**
@@ -64,7 +64,7 @@ int intel_vgpu_register_page_track(struct intel_vgpu *vgpu,
unsigned long gfn,
track->handler = handler;
track->priv_data = priv;
- ret = radix_tree_insert(&vgpu->page_track_tree, gfn, track);
+ ret = xa_err(xa_store(&vgpu->page_track, gfn, track, GFP_KERNEL));
if (ret) {
kfree(track);
return ret;
@@ -84,7 +84,7 @@ void intel_vgpu_unregister_page_track(struct intel_vgpu *vgpu,
{
struct intel_vgpu_page_track *track;
- track = radix_tree_delete(&vgpu->page_track_tree, gfn);
+ track = xa_erase(&vgpu->page_track, gfn);
if (track) {
if (track->tracked)
intel_gvt_hypervisor_disable_page_track(vgpu, gfn);
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index c628be05fbfe..6ec5d16f4e06 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -382,7 +382,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct
intel_gvt *gvt,
mutex_init(&vgpu->vgpu_lock);
mutex_init(&vgpu->dmabuf_lock);
INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head);
- INIT_RADIX_TREE(&vgpu->page_track_tree, GFP_KERNEL);
+ xa_init(&vgpu->page_track);
idr_init(&vgpu->object_idr);
intel_vgpu_init_cfg_space(vgpu, param->primary);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/34] drm: Convert aux_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_dp_aux_dev.c | 41 +---
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index 0e4f25d63fd2..393a32976900 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -49,8 +49,7 @@ struct drm_dp_aux_dev {
#define DRM_AUX_MINORS 256
#define AUX_MAX_OFFSET (1 << 20)
-static DEFINE_IDR(aux_idr);
-static DEFINE_MUTEX(aux_idr_mutex);
+static DEFINE_XARRAY_ALLOC(aux_xa);
static struct class *drm_dp_aux_dev_class;
static int drm_dev_major = -1;
@@ -58,19 +57,21 @@ static struct drm_dp_aux_dev
*drm_dp_aux_dev_get_by_minor(unsigned index)
{
struct drm_dp_aux_dev *aux_dev = NULL;
- mutex_lock(&aux_idr_mutex);
- aux_dev = idr_find(&aux_idr, index);
+ xa_lock(&aux_xa);
+ aux_dev = xa_load(&aux_xa, index);
if (!kref_get_unless_zero(&aux_dev->refcount))
aux_dev = NULL;
- mutex_unlock(&aux_idr_mutex);
+ xa_unlock(&aux_xa);
return aux_dev;
}
static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
{
+ static u32 next;
+
struct drm_dp_aux_dev *aux_dev;
- int index;
+ int err;
aux_dev = kzalloc(sizeof(*aux_dev), GFP_KERNEL);
if (!aux_dev)
@@ -79,15 +80,12 @@ static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct
drm_dp_aux *aux)
atomic_set(&aux_dev->usecount, 1);
kref_init(&aux_dev->refcount);
- mutex_lock(&aux_idr_mutex);
- index = idr_alloc_cyclic(&aux_idr, aux_dev, 0, DRM_AUX_MINORS,
-GFP_KERNEL);
- mutex_unlock(&aux_idr_mutex);
- if (index < 0) {
+ err = xa_alloc_cyclic(&aux_xa, &aux_dev->index, aux_dev,
+ XA_LIMIT(0, DRM_AUX_MINORS), &next, GFP_KERNEL);
+ if (err < 0) {
kfree(aux_dev);
- return ERR_PTR(index);
+ return ERR_PTR(err);
}
- aux_dev->index = index;
return aux_dev;
}
@@ -246,22 +244,19 @@ static const struct file_operations auxdev_fops = {
static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_aux(struct drm_dp_aux *aux)
{
- struct drm_dp_aux_dev *iter, *aux_dev = NULL;
- int id;
+ struct drm_dp_aux_dev *aux_dev;
+ unsigned long id;
/* don't increase kref count here because this function should only be
* used by drm_dp_aux_unregister_devnode. Thus, it will always have at
* least one reference - the one that drm_dp_aux_register_devnode
* created
*/
- mutex_lock(&aux_idr_mutex);
- idr_for_each_entry(&aux_idr, iter, id) {
- if (iter->aux == aux) {
- aux_dev = iter;
+ xa_for_each(&aux_xa, id, aux_dev) {
+ if (aux_dev->aux == aux)
break;
- }
}
- mutex_unlock(&aux_idr_mutex);
+
return aux_dev;
}
@@ -274,9 +269,7 @@ void drm_dp_aux_unregister_devnode(struct drm_dp_aux *aux)
if (!aux_dev) /* attach must have failed */
return;
- mutex_lock(&aux_idr_mutex);
- idr_remove(&aux_idr, aux_dev->index);
- mutex_unlock(&aux_idr_mutex);
+ xa_erase(&aux_xa, aux_dev->index);
atomic_dec(&aux_dev->usecount);
wait_var_event(&aux_dev->usecount, !atomic_read(&aux_dev->usecount));
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/34] drm: Convert object_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 23 +++
drivers/gpu/drm/drm_gem.c | 67 +++
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 ++--
drivers/gpu/drm/i915/i915_debugfs.c | 20 +++---
drivers/gpu/drm/i915/i915_gem_object.h| 2 +-
.../gpu/drm/i915/selftests/i915_gem_context.c | 7 +-
drivers/gpu/drm/msm/msm_gem_submit.c | 12 ++--
drivers/gpu/drm/v3d/v3d_gem.c | 17 +++--
drivers/gpu/drm/vc4/vc4_gem.c | 6 +-
include/drm/drm_file.h| 9 +--
10 files changed, 77 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index f4f00217546e..8c415fdfd828 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -98,16 +98,14 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev)
list_for_each_entry(file, &ddev->filelist, lhead) {
struct drm_gem_object *gobj;
- int handle;
+ unsigned long handle;
WARN_ONCE(1, "Still active user space clients!\n");
- spin_lock(&file->table_lock);
- idr_for_each_entry(&file->object_idr, gobj, handle) {
+ xa_for_each(&file->objects, handle, gobj) {
WARN_ONCE(1, "And also active allocations!\n");
drm_gem_object_put_unlocked(gobj);
}
- idr_destroy(&file->object_idr);
- spin_unlock(&file->table_lock);
+ xa_destroy(&file->objects);
}
mutex_unlock(&ddev->filelist_mutex);
@@ -784,12 +782,10 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
seq_printf((m), " " #flag); \
}
-static int amdgpu_debugfs_gem_bo_info(int id, void *ptr, void *data)
+static int amdgpu_debugfs_gem_bo_info(unsigned int id,
+ struct drm_gem_object *gobj, struct seq_file *m)
{
- struct drm_gem_object *gobj = ptr;
struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
- struct seq_file *m = data;
-
struct dma_buf_attachment *attachment;
struct dma_buf *dma_buf;
unsigned domain;
@@ -851,6 +847,8 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void
*data)
list_for_each_entry(file, &dev->filelist, lhead) {
struct task_struct *task;
+ struct drm_gem_object *gobj;
+ unsigned long index;
/*
* Although we have a valid reference on file->pid, that does
@@ -864,9 +862,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m,
void *data)
task ? task->comm : "");
rcu_read_unlock();
- spin_lock(&file->table_lock);
- idr_for_each(&file->object_idr, amdgpu_debugfs_gem_bo_info, m);
- spin_unlock(&file->table_lock);
+ xa_lock(&file->objects);
+ xa_for_each(&file->objects, index, gobj)
+ amdgpu_debugfs_gem_bo_info(index, gobj, m);
+ xa_unlock(&file->objects);
}
mutex_unlock(&dev->filelist_mutex);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 0a52a958cffe..dc0d3cc3bb35 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -251,10 +251,9 @@ drm_gem_object_handle_put_unlocked(struct drm_gem_object
*obj)
* handle references on objects.
*/
static int
-drm_gem_object_release_handle(int id, void *ptr, void *data)
+drm_gem_object_release_handle(struct drm_gem_object *obj,
+ struct drm_file *file_priv)
{
- struct drm_file *file_priv = data;
- struct drm_gem_object *obj = ptr;
struct drm_device *dev = obj->dev;
if (obj->funcs && obj->funcs->close)
@@ -285,23 +284,17 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
{
struct drm_gem_object *obj;
- spin_lock(&filp->table_lock);
-
/* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- return -EINVAL;
-
- /* Release driver's reference and decrement refcount. */
- drm_gem_object_release_handle(handle, obj, filp);
+ obj = xa_store(&filp->objects, handle, NULL, 0);
+ if (obj) {
+ /* Release driver's reference and decrement refcount. */
+ drm_gem_object_release_handle(obj, filp);
+ }
/* And finally make the handle available for future allocations. */
-
[PATCH 29/34] drm/vc4: Convert perfmon IDR to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
drivers/gpu/drm/vc4/vc4_perfmon.c | 33 +++
2 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 4f87b03f837d..845b84d27f82 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -561,7 +561,7 @@ struct vc4_exec_info {
*/
struct vc4_file {
struct {
- struct idr idr;
+ struct xarray array;
struct mutex lock;
} perfmon;
};
diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
b/drivers/gpu/drm/vc4/vc4_perfmon.c
index 437e7a27f21d..d5485821f996 100644
--- a/drivers/gpu/drm/vc4/vc4_perfmon.c
+++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
@@ -12,9 +12,6 @@
#include "vc4_drv.h"
#include "vc4_regs.h"
-#define VC4_PERFMONID_MIN 1
-#define VC4_PERFMONID_MAX U32_MAX
-
void vc4_perfmon_get(struct vc4_perfmon *perfmon)
{
if (perfmon)
@@ -67,7 +64,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
*vc4file, int id)
struct vc4_perfmon *perfmon;
mutex_lock(&vc4file->perfmon.lock);
- perfmon = idr_find(&vc4file->perfmon.idr, id);
+ perfmon = xa_load(&vc4file->perfmon.array, id);
vc4_perfmon_get(perfmon);
mutex_unlock(&vc4file->perfmon.lock);
@@ -77,23 +74,18 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
*vc4file, int id)
void vc4_perfmon_open_file(struct vc4_file *vc4file)
{
mutex_init(&vc4file->perfmon.lock);
- idr_init(&vc4file->perfmon.idr);
-}
-
-static int vc4_perfmon_idr_del(int id, void *elem, void *data)
-{
- struct vc4_perfmon *perfmon = elem;
-
- vc4_perfmon_put(perfmon);
-
- return 0;
+ xa_init_flags(&vc4file->perfmon.array, XA_FLAGS_ALLOC1);
}
void vc4_perfmon_close_file(struct vc4_file *vc4file)
{
+ struct vc4_perfmon *perfmon;
+ unsigned long index;
+
mutex_lock(&vc4file->perfmon.lock);
- idr_for_each(&vc4file->perfmon.idr, vc4_perfmon_idr_del, NULL);
- idr_destroy(&vc4file->perfmon.idr);
+ xa_for_each(&vc4file->perfmon.array, index, perfmon)
+ vc4_perfmon_put(perfmon);
+ xa_destroy(&vc4file->perfmon.array);
mutex_unlock(&vc4file->perfmon.lock);
}
@@ -130,8 +122,8 @@ int vc4_perfmon_create_ioctl(struct drm_device *dev, void
*data,
refcount_set(&perfmon->refcnt, 1);
mutex_lock(&vc4file->perfmon.lock);
- ret = idr_alloc(&vc4file->perfmon.idr, perfmon, VC4_PERFMONID_MIN,
- VC4_PERFMONID_MAX, GFP_KERNEL);
+ ret = xa_alloc(&vc4file->perfmon.array, &req->id, perfmon,
+ xa_limit_32b, GFP_KERNEL);
mutex_unlock(&vc4file->perfmon.lock);
if (ret < 0) {
@@ -139,7 +131,6 @@ int vc4_perfmon_create_ioctl(struct drm_device *dev, void
*data,
return ret;
}
- req->id = ret;
return 0;
}
@@ -151,7 +142,7 @@ int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void
*data,
struct vc4_perfmon *perfmon;
mutex_lock(&vc4file->perfmon.lock);
- perfmon = idr_remove(&vc4file->perfmon.idr, req->id);
+ perfmon = xa_erase(&vc4file->perfmon.array, req->id);
mutex_unlock(&vc4file->perfmon.lock);
if (!perfmon)
@@ -170,7 +161,7 @@ int vc4_perfmon_get_values_ioctl(struct drm_device *dev,
void *data,
int ret;
mutex_lock(&vc4file->perfmon.lock);
- perfmon = idr_find(&vc4file->perfmon.idr, req->id);
+ perfmon = xa_load(&vc4file->perfmon.array, req->id);
vc4_perfmon_get(perfmon);
mutex_unlock(&vc4file->perfmon.lock);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/34] Convert DRM to XArray
I intend to remove the IDR and the radix tree interfaces from Linux. Converting each user from either the IDR or radix tree interface varies from the trivial 1:1 replacement to a complete rewrite of the locking. Despite the best efforts of our automated testers (who have caught many of my mistakes), I cannot claim that my conversions of code are free from bugs. Please check these patches over carefully and test them; there may be off-by-one errors, locking mistakes, or various other failures on my part. The patches are based on the latest XArray API which can be found here: http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray which I intend to submit during the upcoming merge window. Substantive interface changes ~ - The IDR and radix tree required callers to handle their own locking. The XArray embeds a spinlock which is taken for modifications to the data structure; plain lookups occur under the RCU read lock or under the spinlock. - You can take the spinlock yourself (xa_lock() and friends) to protect related data. - idr_alloc() returned -ENOSPC, radix_tree_insert() returned -EEXIST. xa_insert() and xa_alloc() return -EBUSY. - The search keys which the radix tree calls "tags", the XArray calls "marks". - There is no preloading in the XArray API. If your locking is exceptionally complicated, you may need to use xa_reserve(), but there are only 6 callers of xa_reserve(), so it's quite uncommon. - The radix tree provided GFP flags as part of the tree definition; the XArray (like the IDR) passes GFP flags at the point of allocation. - radix_tree_insert() of a NULL pointer was not well-specified. The XArray treats it as reserving the entry (it reads back as NULL but a subsequent xa_insert() to that slot will fail). - xa_alloc_cyclic() returns 1 if the allocation wraps, unlike idr_alloc_cyclic() which provides no indication. - There is no equivalent to idr_for_each(); the xa_for_each() iterator is similar to idr_for_each_entry(). - idr_replace() has no exact equivalent. Some users relied on its exact semantics of only storing if the entry was non-NULL, but all users of idr_replace() were able to use xa_store(). - The family of radix tree gang lookup functions have been replaced with xa_extract(). Matthew Wilcox (34): drm: Convert drm_minors_idr to XArray drm: Convert aux_idr to XArray drm: Convert object_name_idr to XArray drm: Convert object_idr to XArray drm: Convert syncobj_idr to XArray drm: Convert magic_map to XArray drm: Convert lessee_idr to XArray drm: Remove linked lists for lessees drm: Convert ctx_idr to XArray drm: Convert tile_idr to XArray drm: Convert crtc_idr to XArray drm/agp: Convert bo_list_handles to XArray drm/amdgpu: Convert ctx_handles to XArray drm/amdgpu: Convert pasid_idr to XArray drm/amdkfd: Convert event_idr to XArray drm/amdkfd: Convert alloc_idr to XArray drm/etnaviv: Convert fence_idr to XArray drm/i915: Convert handles_vma to XArray drm/i915: Convert spt_tree to XArray drm/i915: Convert page_track_tree to XArray drm/i915: Convert get_page to XArray drm/i915: Convert object_idr to IDA drm/i915: Convert context_idr to XArray drm/i915: Convert metrics_idr to XArray drm/i915: Convert vgpus_idr to XArray drm/qxl: Convert release_idr to XArray drm/qxl: Convert surf_id_idr to XArray drm/tegra: Convert contexts IDR to XArray drm/vc4: Convert perfmon IDR to XArray drm/sis: Convert object_idr to XArray drm/vgem: Convert fence_idr to XArray drm/via: Convert object_idr to XArray drm/vmwgfx: Convert base IDR to XArray drm/vmwgfx: Convert res_idr to XArray Documentation/gpu/todo.rst| 3 - drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 22 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 42 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 23 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 66 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 3 +- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 12 +- drivers/gpu/drm/amd/amdkfd/kfd_events.c | 71 +++ drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 4 +- drivers/gpu/drm/amd/amdkfd/kfd_process.c | 32 ++- drivers/gpu/drm/drm_auth.c| 27 +-- drivers/gpu/drm/drm_connector.c | 27 +-- drivers/gpu/drm/drm_context.c | 42 ++-- drivers/gpu/drm/drm_debugfs.c | 19 +- drivers/gpu/drm/drm_dp_aux_dev.c | 41 ++-- drivers/gpu/drm/drm_drv.c | 49 ++--- drivers/gpu/drm/drm_gem.c | 78 +++ drivers/gpu/drm/drm_lease.
[PATCH 34/34] drm/vmwgfx: Convert res_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 9 +--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 31 ++--
3 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 25afb1d594e3..2d121fbe5c93 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -677,7 +677,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned
long chipset)
spin_lock_init(&dev_priv->cursor_lock);
for (i = vmw_res_context; i < vmw_res_max; ++i) {
- idr_init(&dev_priv->res_idr[i]);
+ xa_init_flags(&dev_priv->resources[i], XA_FLAGS_ALLOC1);
INIT_LIST_HEAD(&dev_priv->res_lru[i]);
}
@@ -988,9 +988,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned
long chipset)
out_err4:
memunmap(dev_priv->mmio_virt);
out_err0:
- for (i = vmw_res_context; i < vmw_res_max; ++i)
- idr_destroy(&dev_priv->res_idr[i]);
-
if (dev_priv->ctx.staged_bindings)
vmw_binding_state_free(dev_priv->ctx.staged_bindings);
kfree(dev_priv);
@@ -1000,7 +997,6 @@ static int vmw_driver_load(struct drm_device *dev,
unsigned long chipset)
static void vmw_driver_unload(struct drm_device *dev)
{
struct vmw_private *dev_priv = vmw_priv(dev);
- enum vmw_res_type i;
unregister_pm_notifier(&dev_priv->pm_nb);
@@ -1039,9 +1035,6 @@ static void vmw_driver_unload(struct drm_device *dev)
if (dev_priv->ctx.staged_bindings)
vmw_binding_state_free(dev_priv->ctx.staged_bindings);
- for (i = vmw_res_context; i < vmw_res_max; ++i)
- idr_destroy(&dev_priv->res_idr[i]);
-
kfree(dev_priv);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index cd607ba9c2fe..e7613a33a773 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -484,7 +484,8 @@ struct vmw_private {
*/
spinlock_t resource_lock;
- struct idr res_idr[vmw_res_max];
+ struct xarray resources[vmw_res_max];
+
/*
* Block lastclose from racing with firstopen.
*/
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 3025bfc001a1..d8cc329a9c0e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -56,13 +56,11 @@ vmw_resource_reference_unless_doomed(struct vmw_resource
*res)
void vmw_resource_release_id(struct vmw_resource *res)
{
struct vmw_private *dev_priv = res->dev_priv;
- struct idr *idr = &dev_priv->res_idr[res->func->res_type];
+ struct xarray *xa = &dev_priv->resources[res->func->res_type];
- spin_lock(&dev_priv->resource_lock);
if (res->id != -1)
- idr_remove(idr, res->id);
+ xa_erase(xa, res->id);
res->id = -1;
- spin_unlock(&dev_priv->resource_lock);
}
static void vmw_resource_release(struct kref *kref)
@@ -70,8 +68,7 @@ static void vmw_resource_release(struct kref *kref)
struct vmw_resource *res =
container_of(kref, struct vmw_resource, kref);
struct vmw_private *dev_priv = res->dev_priv;
- int id;
- struct idr *idr = &dev_priv->res_idr[res->func->res_type];
+ struct xarray *xa = &dev_priv->resources[res->func->res_type];
spin_lock(&dev_priv->resource_lock);
list_del_init(&res->lru_head);
@@ -101,16 +98,12 @@ static void vmw_resource_release(struct kref *kref)
res->hw_destroy(res);
}
- id = res->id;
+ if (res->id != -1)
+ xa_erase(xa, res->id);
if (res->res_free != NULL)
res->res_free(res);
else
kfree(res);
-
- spin_lock(&dev_priv->resource_lock);
- if (id != -1)
- idr_remove(idr, id);
- spin_unlock(&dev_priv->resource_lock);
}
void vmw_resource_unreference(struct vmw_resource **p_res)
@@ -133,21 +126,11 @@ void vmw_resource_unreference(struct vmw_resource **p_res)
int vmw_resource_alloc_id(struct vmw_resource *res)
{
struct vmw_private *dev_priv = res->dev_priv;
- int ret;
- struct idr *idr = &dev_priv->res_idr[res->func->res_type];
+ struct xarray *xa = &dev_priv->resources[res->func->res_type];
BUG_ON(res->id != -1);
- idr_preload(GFP_KERNEL);
- spin_lock(&dev_priv->resource_lock);
-
- ret = idr_alloc(idr, res, 1, 0, GFP_NOWAIT);
- if
[PATCH 03/34] drm: Convert object_name_idr to XArray
Leave the object_name_lock in place for now as I'm not certain it can be
removed safely.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_debugfs.c | 19 ++-
drivers/gpu/drm/drm_gem.c | 11 +--
include/drm/drm_device.h | 2 +-
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index f8468eae0503..2bf08f293331 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -106,27 +106,20 @@ static int drm_clients_info(struct seq_file *m, void
*data)
return 0;
}
-static int drm_gem_one_name_info(int id, void *ptr, void *data)
-{
- struct drm_gem_object *obj = ptr;
- struct seq_file *m = data;
-
- seq_printf(m, "%6d %8zd %7d %8d\n",
- obj->name, obj->size,
- obj->handle_count,
- kref_read(&obj->refcount));
- return 0;
-}
-
static int drm_gem_name_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
+ struct drm_gem_object *obj;
+ unsigned long index;
seq_printf(m, " name size handles refcount\n");
mutex_lock(&dev->object_name_lock);
- idr_for_each(&dev->object_name_idr, drm_gem_one_name_info, m);
+ xa_for_each(&dev->object_names, index, obj) {
+ seq_printf(m, "%6d %8zd %7d %8d\n", obj->name, obj->size,
+ obj->handle_count, kref_read(&obj->refcount));
+ }
mutex_unlock(&dev->object_name_lock);
return 0;
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 8b55ece97967..0a52a958cffe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -98,7 +98,7 @@ drm_gem_init(struct drm_device *dev)
struct drm_vma_offset_manager *vma_offset_manager;
mutex_init(&dev->object_name_lock);
- idr_init_base(&dev->object_name_idr, 1);
+ xa_init_flags(&dev->object_names, XA_FLAGS_ALLOC1);
vma_offset_manager = kzalloc(sizeof(*vma_offset_manager), GFP_KERNEL);
if (!vma_offset_manager) {
@@ -205,7 +205,7 @@ static void drm_gem_object_handle_free(struct
drm_gem_object *obj)
/* Remove any name for this object */
if (obj->name) {
- idr_remove(&dev->object_name_idr, obj->name);
+ xa_erase(&dev->object_names, obj->name);
obj->name = 0;
}
}
@@ -714,11 +714,10 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
}
if (!obj->name) {
- ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_KERNEL);
+ ret = xa_alloc(&dev->object_names, &obj->name, obj,
+ xa_limit_32b, GFP_KERNEL);
if (ret < 0)
goto err;
-
- obj->name = ret;
}
args->name = (uint64_t) obj->name;
@@ -754,7 +753,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
return -EOPNOTSUPP;
mutex_lock(&dev->object_name_lock);
- obj = idr_find(&dev->object_name_idr, (int) args->name);
+ obj = xa_load(&dev->object_names, (int) args->name);
if (obj) {
drm_gem_object_get(obj);
} else {
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 42411b3ea0c8..52e271b97de8 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -219,7 +219,7 @@ struct drm_device {
/** \name GEM information */
/*@{ */
struct mutex object_name_lock;
- struct idr object_name_idr;
+ struct xarray object_names;
struct drm_vma_offset_manager *vma_offset_manager;
/*@} */
int switch_power_state;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 30/34] drm/sis: Convert object_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/sis/sis_drv.c | 4 +---
drivers/gpu/drm/sis/sis_drv.h | 2 +-
drivers/gpu/drm/sis/sis_mm.c | 17 -
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index e04a92658cd7..bb5caad5d365 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -47,7 +47,7 @@ static int sis_driver_load(struct drm_device *dev, unsigned
long chipset)
if (dev_priv == NULL)
return -ENOMEM;
- idr_init(&dev_priv->object_idr);
+ xa_init_flags(&dev_priv->objects, XA_FLAGS_ALLOC1);
dev->dev_private = (void *)dev_priv;
dev_priv->chipset = chipset;
@@ -58,8 +58,6 @@ static void sis_driver_unload(struct drm_device *dev)
{
drm_sis_private_t *dev_priv = dev->dev_private;
- idr_destroy(&dev_priv->object_idr);
-
kfree(dev_priv);
}
diff --git a/drivers/gpu/drm/sis/sis_drv.h b/drivers/gpu/drm/sis/sis_drv.h
index 328f8a750976..18277fee8550 100644
--- a/drivers/gpu/drm/sis/sis_drv.h
+++ b/drivers/gpu/drm/sis/sis_drv.h
@@ -64,7 +64,7 @@ typedef struct drm_sis_private {
struct drm_mm vram_mm;
struct drm_mm agp_mm;
/** Mapping of userspace keys to mm objects */
- struct idr object_idr;
+ struct xarray objects;
} drm_sis_private_t;
struct sis_file_private {
diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c
index 1622db24cd39..2fbc31697563 100644
--- a/drivers/gpu/drm/sis/sis_mm.c
+++ b/drivers/gpu/drm/sis/sis_mm.c
@@ -84,9 +84,10 @@ static int sis_drm_alloc(struct drm_device *dev, struct
drm_file *file,
{
drm_sis_private_t *dev_priv = dev->dev_private;
drm_sis_mem_t *mem = data;
- int retval = 0, user_key;
+ int retval = 0;
struct sis_memblock *item;
struct sis_file_private *file_priv = file->driver_priv;
+ unsigned int id;
unsigned long offset;
mutex_lock(&dev->struct_mutex);
@@ -128,23 +129,22 @@ static int sis_drm_alloc(struct drm_device *dev, struct
drm_file *file,
if (retval)
goto fail_alloc;
- retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL);
+ retval = xa_alloc(&dev_priv->objects, &id, item, xa_limit_31b,
+ GFP_KERNEL);
if (retval < 0)
- goto fail_idr;
- user_key = retval;
-
+ goto fail_xa;
list_add(&item->owner_list, &file_priv->obj_list);
mutex_unlock(&dev->struct_mutex);
+ mem->free = id;
mem->offset = ((pool == 0) ?
dev_priv->vram_offset : dev_priv->agp_offset) +
(offset << SIS_MM_ALIGN_SHIFT);
- mem->free = user_key;
mem->size = mem->size << SIS_MM_ALIGN_SHIFT;
return 0;
-fail_idr:
+fail_xa:
drm_mm_remove_node(&item->mm_node);
fail_alloc:
kfree(item);
@@ -167,13 +167,12 @@ static int sis_drm_free(struct drm_device *dev, void
*data, struct drm_file *fil
struct sis_memblock *obj;
mutex_lock(&dev->struct_mutex);
- obj = idr_find(&dev_priv->object_idr, mem->free);
+ obj = xa_erase(&dev_priv->objects, mem->free);
if (obj == NULL) {
mutex_unlock(&dev->struct_mutex);
return -EINVAL;
}
- idr_remove(&dev_priv->object_idr, mem->free);
list_del(&obj->owner_list);
if (drm_mm_node_allocated(&obj->mm_node))
drm_mm_remove_node(&obj->mm_node);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/34] drm/amdgpu: Convert pasid_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +-
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 12 ++---
3 files changed, 29 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d2ea5ce2cefb..d094ec7433f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3032,12 +3032,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
amdgpu_bo_unreserve(vm->root.base.bo);
if (pasid) {
- unsigned long flags;
-
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1,
- GFP_ATOMIC);
- spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
+ r = xa_insert_irq(&adev->vm_manager.vms, pasid, vm, GFP_KERNEL);
if (r < 0)
goto error_free_root;
@@ -3047,6 +3042,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
vm->fault_hash = init_fault_hash();
if (!vm->fault_hash) {
r = -ENOMEM;
+ if (pasid)
+ xa_erase_irq(&adev->vm_manager.vms, pasid);
goto error_free_root;
}
@@ -3104,16 +3101,9 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm, uns
}
if (pasid) {
- unsigned long flags;
-
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1,
- GFP_ATOMIC);
- spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
-
- if (r == -ENOSPC)
+ r = xa_insert_irq(&adev->vm_manager.vms, pasid, vm, GFP_KERNEL);
+ if (r < 0)
goto unreserve_bo;
- r = 0;
}
/* Check if PD needs to be reinitialized and do it before
@@ -3124,7 +3114,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm, uns
adev->vm_manager.root_level,
pte_support_ats);
if (r)
- goto free_idr;
+ goto erase;
}
/* Update VM state */
@@ -3137,11 +3127,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm, uns
"CPU update of VM recommended only for large BAR system\n");
if (vm->pasid) {
- unsigned long flags;
-
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
- spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
+ xa_erase_irq(&adev->vm_manager.vms, vm->pasid);
/* Free the original amdgpu allocated pasid
* Will be replaced with kfd allocated pasid
@@ -3158,14 +3144,9 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm, uns
goto unreserve_bo;
-free_idr:
- if (pasid) {
- unsigned long flags;
-
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- idr_remove(&adev->vm_manager.pasid_idr, pasid);
- spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
- }
+erase:
+ if (pasid)
+ xa_erase_irq(&adev->vm_manager.vms, pasid);
unreserve_bo:
amdgpu_bo_unreserve(vm->root.base.bo);
return r;
@@ -3184,9 +3165,9 @@ void amdgpu_vm_release_compute(struct amdgpu_device
*adev, struct amdgpu_vm *vm)
if (vm->pasid) {
unsigned long flags;
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
- spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
+ xa_lock_irqsave(&adev->vm_manager.vms, flags);
+ __xa_erase(&adev->vm_manager.vms, vm->pasid);
+ xa_unlock_irqrestore(&adev->vm_manager.vms, flags);
}
vm->pasid = 0;
}
@@ -3217,9 +3198,9 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
if (vm->pasid) {
unsigned long flags;
- spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
- idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
- spin_unlock_irqres
[PATCH 01/34] drm: Convert drm_minors_idr to XArray
Divide all the indices by 64 to save memory.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_drv.c | 49 ++-
1 file changed, 17 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 12e5e2be7890..17ed29f49060 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -64,8 +64,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit
enables a debug cat
"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
module_param_named(debug, drm_debug, int, 0600);
-static DEFINE_SPINLOCK(drm_minor_lock);
-static struct idr drm_minors_idr;
+static DEFINE_XARRAY_FLAGS(drm_minors, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
/*
* If the drm core fails to init for whatever reason,
@@ -109,7 +108,6 @@ static struct drm_minor **drm_minor_get_slot(struct
drm_device *dev,
static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
{
struct drm_minor *minor;
- unsigned long flags;
int r;
minor = kzalloc(sizeof(*minor), GFP_KERNEL);
@@ -118,22 +116,12 @@ static int drm_minor_alloc(struct drm_device *dev,
unsigned int type)
minor->type = type;
minor->dev = dev;
+ minor->index = 64 * type;
- idr_preload(GFP_KERNEL);
- spin_lock_irqsave(&drm_minor_lock, flags);
- r = idr_alloc(&drm_minors_idr,
- NULL,
- 64 * type,
- 64 * (type + 1),
- GFP_NOWAIT);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
- idr_preload_end();
-
+ r = xa_insert_irq(&drm_minors, minor->index / 64, NULL, GFP_KERNEL);
if (r < 0)
goto err_free;
- minor->index = r;
-
minor->kdev = drm_sysfs_minor_alloc(minor);
if (IS_ERR(minor->kdev)) {
r = PTR_ERR(minor->kdev);
@@ -144,9 +132,7 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned
int type)
return 0;
err_index:
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_remove(&drm_minors_idr, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ xa_erase_irq(&drm_minors, minor->index / 64);
err_free:
kfree(minor);
return r;
@@ -164,9 +150,9 @@ static void drm_minor_free(struct drm_device *dev, unsigned
int type)
put_device(minor->kdev);
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_remove(&drm_minors_idr, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ xa_lock_irqsave(&drm_minors, flags);
+ __xa_erase(&drm_minors, minor->index / 64);
+ xa_unlock_irqrestore(&drm_minors, flags);
kfree(minor);
*slot = NULL;
@@ -195,9 +181,9 @@ static int drm_minor_register(struct drm_device *dev,
unsigned int type)
goto err_debugfs;
/* replace NULL with @minor so lookups will succeed from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_replace(&drm_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ xa_lock_irqsave(&drm_minors, flags);
+ __xa_store(&drm_minors, minor->index / 64, minor, 0);
+ xa_unlock_irqrestore(&drm_minors, flags);
DRM_DEBUG("new minor registered %d\n", minor->index);
return 0;
@@ -217,9 +203,9 @@ static void drm_minor_unregister(struct drm_device *dev,
unsigned int type)
return;
/* replace @minor with NULL so lookups will fail from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_replace(&drm_minors_idr, NULL, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ xa_lock_irqsave(&drm_minors, flags);
+ __xa_store(&drm_minors, minor->index / 64, NULL, 0);
+ xa_unlock_irqrestore(&drm_minors, flags);
device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
@@ -240,11 +226,11 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
struct drm_minor *minor;
unsigned long flags;
- spin_lock_irqsave(&drm_minor_lock, flags);
- minor = idr_find(&drm_minors_idr, minor_id);
+ xa_lock_irqsave(&drm_minors, flags);
+ minor = xa_load(&drm_minors, minor_id / 64);
if (minor)
drm_dev_get(minor->dev);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ xa_unlock_irqrestore(&drm_minors, flags);
if (!minor) {
return ERR_PTR(-ENODEV);
@@ -958,7 +944,7 @@ static void drm_core_exit(void)
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
drm_sysfs_destroy();
[PATCH 17/34] drm/etnaviv: Convert fence_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 4 +---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 5 ++---
drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 3 ++-
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 8
4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 98f803510e0a..f950542290ca 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -388,9 +388,7 @@ static void submit_cleanup(struct kref *kref)
dma_fence_put(submit->in_fence);
if (submit->out_fence) {
/* first remove from IDR, so fence can not be found anymore */
- mutex_lock(&submit->gpu->fence_lock);
- idr_remove(&submit->gpu->fence_idr, submit->out_fence_id);
- mutex_unlock(&submit->gpu->fence_lock);
+ xa_erase(&submit->gpu->fences, submit->out_fence_id);
dma_fence_put(submit->out_fence);
}
kfree(submit->pmrs);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 6904535475de..b1d407308680 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1147,7 +1147,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct
etnaviv_gpu *gpu,
* pretends we didn't find a fence in that case.
*/
rcu_read_lock();
- fence = idr_find(&gpu->fence_idr, id);
+ fence = xa_load(&gpu->fences, id);
if (fence)
fence = dma_fence_get_rcu(fence);
rcu_read_unlock();
@@ -1630,7 +1630,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct
device *master,
gpu->drm = drm;
gpu->fence_context = dma_fence_context_alloc(1);
- idr_init(&gpu->fence_idr);
+ xa_init_flags(&gpu->fences, XA_FLAGS_ALLOC);
spin_lock_init(&gpu->fence_spinlock);
INIT_WORK(&gpu->sync_point_work, sync_point_worker);
@@ -1689,7 +1689,6 @@ static void etnaviv_gpu_unbind(struct device *dev, struct
device *master,
}
gpu->drm = NULL;
- idr_destroy(&gpu->fence_idr);
if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL))
thermal_cooling_device_unregister(gpu->cooling);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 9bcf151f706b..d87406ffecb5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -117,7 +117,8 @@ struct etnaviv_gpu {
/* Fencing support */
struct mutex fence_lock;
- struct idr fence_idr;
+ struct xarray fences;
+ u32 fences_next;/* For the XArray allocation */
u32 next_fence;
u32 completed_fence;
wait_queue_head_t fence_event;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 49a6763693f1..f515e671ff07 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -155,10 +155,10 @@ int etnaviv_sched_push_job(struct drm_sched_entity
*sched_entity,
goto out_unlock;
submit->out_fence = dma_fence_get(&submit->sched_job.s_fence->finished);
- submit->out_fence_id = idr_alloc_cyclic(&submit->gpu->fence_idr,
- submit->out_fence, 0,
- INT_MAX, GFP_KERNEL);
- if (submit->out_fence_id < 0) {
+ ret = xa_alloc_cyclic(&submit->gpu->fences, &submit->out_fence_id,
+ submit->out_fence, xa_limit_31b,
+ &submit->gpu->fences_next, GFP_KERNEL);
+ if (ret < 0) {
drm_sched_job_cleanup(&submit->sched_job);
ret = -ENOMEM;
goto out_unlock;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 28/34] drm/tegra: Convert contexts IDR to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/tegra/drm.c | 35 ++-
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 4b70ce664c41..5daa414cc2cf 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -9,7 +9,7 @@
#include
#include
-#include
+#include
#include
#include
@@ -33,7 +33,7 @@
#define CDMA_GATHER_FETCHES_MAX_NB 16383
struct tegra_drm_file {
- struct idr contexts;
+ struct xarray contexts;
struct mutex lock;
};
@@ -246,7 +246,7 @@ static int tegra_drm_open(struct drm_device *drm, struct
drm_file *filp)
if (!fpriv)
return -ENOMEM;
- idr_init(&fpriv->contexts);
+ xa_init_flags(&fpriv->contexts, XA_FLAGS_ALLOC1);
mutex_init(&fpriv->lock);
filp->driver_priv = fpriv;
@@ -582,14 +582,14 @@ static int tegra_client_open(struct tegra_drm_file *fpriv,
if (err < 0)
return err;
- err = idr_alloc(&fpriv->contexts, context, 1, 0, GFP_KERNEL);
+ err = xa_alloc(&fpriv->contexts, &context->id, context, xa_limit_31b,
+ GFP_KERNEL);
if (err < 0) {
client->ops->close_channel(context);
return err;
}
context->client = client;
- context->id = err;
return 0;
}
@@ -637,13 +637,12 @@ static int tegra_close_channel(struct drm_device *drm,
void *data,
mutex_lock(&fpriv->lock);
- context = idr_find(&fpriv->contexts, args->context);
+ context = xa_erase(&fpriv->contexts, args->context);
if (!context) {
err = -EINVAL;
goto unlock;
}
- idr_remove(&fpriv->contexts, context->id);
tegra_drm_context_free(context);
unlock:
@@ -662,7 +661,7 @@ static int tegra_get_syncpt(struct drm_device *drm, void
*data,
mutex_lock(&fpriv->lock);
- context = idr_find(&fpriv->contexts, args->context);
+ context = xa_load(&fpriv->contexts, args->context);
if (!context) {
err = -ENODEV;
goto unlock;
@@ -691,7 +690,7 @@ static int tegra_submit(struct drm_device *drm, void *data,
mutex_lock(&fpriv->lock);
- context = idr_find(&fpriv->contexts, args->context);
+ context = xa_load(&fpriv->contexts, args->context);
if (!context) {
err = -ENODEV;
goto unlock;
@@ -716,7 +715,7 @@ static int tegra_get_syncpt_base(struct drm_device *drm,
void *data,
mutex_lock(&fpriv->lock);
- context = idr_find(&fpriv->contexts, args->context);
+ context = xa_load(&fpriv->contexts, args->context);
if (!context) {
err = -ENODEV;
goto unlock;
@@ -928,24 +927,18 @@ static const struct file_operations tegra_drm_fops = {
.llseek = noop_llseek,
};
-static int tegra_drm_context_cleanup(int id, void *p, void *data)
-{
- struct tegra_drm_context *context = p;
-
- tegra_drm_context_free(context);
-
- return 0;
-}
-
static void tegra_drm_postclose(struct drm_device *drm, struct drm_file *file)
{
struct tegra_drm_file *fpriv = file->driver_priv;
+ struct tegra_drm_context *context;
+ unsigned long index;
mutex_lock(&fpriv->lock);
- idr_for_each(&fpriv->contexts, tegra_drm_context_cleanup, NULL);
+ xa_for_each(&fpriv->contexts, index, context)
+ tegra_drm_context_free(context);
mutex_unlock(&fpriv->lock);
- idr_destroy(&fpriv->contexts);
+ xa_destroy(&fpriv->contexts);
mutex_destroy(&fpriv->lock);
kfree(fpriv);
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 33/34] drm/vmwgfx: Convert base IDR to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/vmwgfx/ttm_object.c | 29 -
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c
b/drivers/gpu/drm/vmwgfx/ttm_object.c
index 36990b80e790..8f394b94060b 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -77,8 +77,6 @@ struct ttm_object_file {
/**
* struct ttm_object_device
*
- * @object_lock: lock that protects the object_hash hash table.
- *
* @object_hash: hash table for fast lookup of object global names.
*
* @object_count: Per device object count.
@@ -87,14 +85,13 @@ struct ttm_object_file {
*/
struct ttm_object_device {
- spinlock_t object_lock;
struct drm_open_hash object_hash;
atomic_t object_count;
struct ttm_mem_global *mem_glob;
struct dma_buf_ops ops;
void (*dmabuf_release)(struct dma_buf *dma_buf);
size_t dma_buf_size;
- struct idr idr;
+ struct xarray bases;
};
/**
@@ -172,15 +169,11 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
base->ref_obj_release = ref_obj_release;
base->object_type = object_type;
kref_init(&base->refcount);
- idr_preload(GFP_KERNEL);
- spin_lock(&tdev->object_lock);
- ret = idr_alloc(&tdev->idr, base, 0, 0, GFP_NOWAIT);
- spin_unlock(&tdev->object_lock);
- idr_preload_end();
+ ret = xa_alloc(&tdev->bases, &base->handle, base, xa_limit_31b,
+ GFP_KERNEL);
if (ret < 0)
return ret;
- base->handle = ret;
ret = ttm_ref_object_add(tfile, base, TTM_REF_USAGE, NULL, false);
if (unlikely(ret != 0))
goto out_err1;
@@ -189,9 +182,7 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
return 0;
out_err1:
- spin_lock(&tdev->object_lock);
- idr_remove(&tdev->idr, base->handle);
- spin_unlock(&tdev->object_lock);
+ xa_erase(&tdev->bases, base->handle);
return ret;
}
@@ -201,9 +192,7 @@ static void ttm_release_base(struct kref *kref)
container_of(kref, struct ttm_base_object, refcount);
struct ttm_object_device *tdev = base->tfile->tdev;
- spin_lock(&tdev->object_lock);
- idr_remove(&tdev->idr, base->handle);
- spin_unlock(&tdev->object_lock);
+ xa_erase(&tdev->bases, base->handle);
/*
* Note: We don't use synchronize_rcu() here because it's far
@@ -287,7 +276,7 @@ ttm_base_object_lookup_for_ref(struct ttm_object_device
*tdev, uint32_t key)
struct ttm_base_object *base;
rcu_read_lock();
- base = idr_find(&tdev->idr, key);
+ base = xa_load(&tdev->bases, key);
if (base && !kref_get_unless_zero(&base->refcount))
base = NULL;
@@ -534,13 +523,12 @@ ttm_object_device_init(struct ttm_mem_global *mem_glob,
return NULL;
tdev->mem_glob = mem_glob;
- spin_lock_init(&tdev->object_lock);
atomic_set(&tdev->object_count, 0);
ret = drm_ht_create(&tdev->object_hash, hash_order);
if (ret != 0)
goto out_no_object_hash;
- idr_init(&tdev->idr);
+ xa_init_flags(&tdev->bases, XA_FLAGS_ALLOC);
tdev->ops = *ops;
tdev->dmabuf_release = tdev->ops.release;
tdev->ops.release = ttm_prime_dmabuf_release;
@@ -559,8 +547,7 @@ void ttm_object_device_release(struct ttm_object_device
**p_tdev)
*p_tdev = NULL;
- WARN_ON_ONCE(!idr_is_empty(&tdev->idr));
- idr_destroy(&tdev->idr);
+ WARN_ON_ONCE(!xa_empty(&tdev->bases));
drm_ht_remove(&tdev->object_hash);
kfree(tdev);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 19/34] drm/i915: Convert spt_tree to XArray
A straightforward conversion.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/gvt/gtt.c | 18 --
drivers/gpu/drm/i915/gvt/gtt.h | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index c7103dd2d8d5..4b8468839312 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -735,7 +735,7 @@ static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt *spt)
dma_unmap_page(kdev, spt->shadow_page.mfn << I915_GTT_PAGE_SHIFT, 4096,
PCI_DMA_BIDIRECTIONAL);
- radix_tree_delete(&spt->vgpu->gtt.spt_tree, spt->shadow_page.mfn);
+ xa_erase(&spt->vgpu->gtt.spts, spt->shadow_page.mfn);
if (spt->guest_page.gfn) {
if (spt->guest_page.oos_page)
@@ -751,11 +751,9 @@ static void ppgtt_free_spt(struct intel_vgpu_ppgtt_spt
*spt)
static void ppgtt_free_all_spt(struct intel_vgpu *vgpu)
{
struct intel_vgpu_ppgtt_spt *spt;
- struct radix_tree_iter iter;
- void **slot;
+ unsigned long index;
- radix_tree_for_each_slot(slot, &vgpu->gtt.spt_tree, &iter, 0) {
- spt = radix_tree_deref_slot(slot);
+ xa_for_each(&vgpu->gtt.spts, index, spt) {
ppgtt_free_spt(spt);
}
}
@@ -798,7 +796,7 @@ static struct intel_vgpu_ppgtt_spt
*intel_vgpu_find_spt_by_gfn(
static inline struct intel_vgpu_ppgtt_spt *intel_vgpu_find_spt_by_mfn(
struct intel_vgpu *vgpu, unsigned long mfn)
{
- return radix_tree_lookup(&vgpu->gtt.spt_tree, mfn);
+ return xa_load(&vgpu->gtt.spts, mfn);
}
static int reclaim_one_ppgtt_mm(struct intel_gvt *gvt);
@@ -840,13 +838,13 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
spt->shadow_page.vaddr = page_address(spt->shadow_page.page);
spt->shadow_page.mfn = daddr >> I915_GTT_PAGE_SHIFT;
- ret = radix_tree_insert(&vgpu->gtt.spt_tree, spt->shadow_page.mfn, spt);
- if (ret)
+ if (xa_store(&vgpu->gtt.spts, spt->shadow_page.mfn, spt, GFP_KERNEL))
goto err_unmap_dma;
return spt;
err_unmap_dma:
+ ret = -ENOMEM;
dma_unmap_page(kdev, daddr, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
err_free_spt:
free_spt(spt);
@@ -2407,7 +2405,7 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
{
struct intel_vgpu_gtt *gtt = &vgpu->gtt;
- INIT_RADIX_TREE(>t->spt_tree, GFP_KERNEL);
+ xa_init(>t->spts);
INIT_LIST_HEAD(>t->ppgtt_mm_list_head);
INIT_LIST_HEAD(>t->oos_page_list_head);
@@ -2439,7 +2437,7 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct
intel_vgpu *vgpu)
if (GEM_WARN_ON(!list_empty(&vgpu->gtt.ppgtt_mm_list_head)))
gvt_err("vgpu ppgtt mm is not fully destroyed\n");
- if (GEM_WARN_ON(!radix_tree_empty(&vgpu->gtt.spt_tree))) {
+ if (GEM_WARN_ON(!xa_empty(&vgpu->gtt.spts))) {
gvt_err("Why we still has spt not freed?\n");
ppgtt_free_all_spt(vgpu);
}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index d8cb04cc946d..b7a858aab356 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -198,7 +198,7 @@ struct intel_vgpu_gtt {
struct intel_vgpu_mm *ggtt_mm;
unsigned long active_ppgtt_mm_bitmap;
struct list_head ppgtt_mm_list_head;
- struct radix_tree_root spt_tree;
+ struct xarray spts;
struct list_head oos_page_list_head;
struct list_head post_shadow_list_head;
struct intel_vgpu_scratch_pt scratch_pt[GTT_TYPE_MAX];
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1c31967194b..4b5ce517cbcf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include
#include
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 16/34] drm/amdkfd: Convert alloc_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h| 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 32 +++-
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 9878abc6d847..4f67cb707b7b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -590,7 +590,7 @@ struct kfd_process_device {
void *vm;
/* GPUVM allocations storage */
- struct idr alloc_idr;
+ struct xarray allocations;
/* Flag used to tell the pdd has dequeued from the dqm.
* This is used to prevent dev->dqm->ops.process_termination() from
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 80b36e860a0a..1ed34d0e9561 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -284,13 +284,13 @@ static void kfd_process_device_free_bos(struct
kfd_process_device *pdd)
{
struct kfd_process *p = pdd->process;
void *mem;
- int id;
+ unsigned long id;
/*
-* Remove all handles from idr and release appropriate
+* Remove all handles and release appropriate
* local memory object
*/
- idr_for_each_entry(&pdd->alloc_idr, mem, id) {
+ xa_for_each(&pdd->allocations, id, mem) {
struct kfd_process_device *peer_pdd;
list_for_each_entry(peer_pdd, &p->per_device_data,
@@ -339,8 +339,6 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
get_order(KFD_CWSR_TBA_TMA_SIZE));
kfree(pdd->qpd.doorbell_bitmap);
- idr_destroy(&pdd->alloc_idr);
-
kfree(pdd);
}
}
@@ -656,8 +654,7 @@ struct kfd_process_device
*kfd_create_process_device_data(struct kfd_dev *dev,
pdd->already_dequeued = false;
list_add(&pdd->per_device_list, &p->per_device_data);
- /* Init idr used for memory handle translation */
- idr_init(&pdd->alloc_idr);
+ xa_init_flags(&pdd->allocations, XA_FLAGS_ALLOC);
return pdd;
}
@@ -774,35 +771,36 @@ bool kfd_has_process_device_data(struct kfd_process *p)
return !(list_empty(&p->per_device_data));
}
-/* Create specific handle mapped to mem from process local memory idr
+/* Create specific handle mapped to mem from process local memory
* Assumes that the process lock is held.
*/
int kfd_process_device_create_obj_handle(struct kfd_process_device *pdd,
void *mem)
{
- return idr_alloc(&pdd->alloc_idr, mem, 0, 0, GFP_KERNEL);
+ int id, ret;
+
+ ret = xa_alloc(&pdd->allocations, &id, mem, xa_limit_31b, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ return id;
}
-/* Translate specific handle from process local memory idr
+/* Translate specific handle from process local memory
* Assumes that the process lock is held.
*/
void *kfd_process_device_translate_handle(struct kfd_process_device *pdd,
int handle)
{
- if (handle < 0)
- return NULL;
-
- return idr_find(&pdd->alloc_idr, handle);
+ return xa_load(&pdd->allocations, handle);
}
-/* Remove specific handle from process local memory idr
+/* Remove specific handle from process local memory
* Assumes that the process lock is held.
*/
void kfd_process_device_remove_obj_handle(struct kfd_process_device *pdd,
int handle)
{
- if (handle >= 0)
- idr_remove(&pdd->alloc_idr, handle);
+ xa_erase(&pdd->allocations, handle);
}
/* This increments the process->ref counter. */
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 27/34] drm/qxl: Convert surf_id_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/qxl/qxl_cmd.c | 60 ---
drivers/gpu/drm/qxl/qxl_drv.h | 3 +-
drivers/gpu/drm/qxl/qxl_kms.c | 5 +--
3 files changed, 23 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index dffc5093ff16..afe8079453af 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -420,41 +420,27 @@ void qxl_io_monitors_config(struct qxl_device *qdev)
int qxl_surface_id_alloc(struct qxl_device *qdev,
struct qxl_bo *surf)
{
- uint32_t handle;
- int idr_ret;
- int count = 0;
+ int ret;
again:
- idr_preload(GFP_ATOMIC);
- spin_lock(&qdev->surf_id_idr_lock);
- idr_ret = idr_alloc(&qdev->surf_id_idr, NULL, 1, 0, GFP_NOWAIT);
- spin_unlock(&qdev->surf_id_idr_lock);
- idr_preload_end();
- if (idr_ret < 0)
- return idr_ret;
- handle = idr_ret;
-
- if (handle >= qdev->rom->n_surfaces) {
- count++;
- spin_lock(&qdev->surf_id_idr_lock);
- idr_remove(&qdev->surf_id_idr, handle);
- spin_unlock(&qdev->surf_id_idr_lock);
+ ret = xa_alloc(&qdev->surfaces, &surf->surface_id, NULL,
+ XA_LIMIT(0, qdev->rom->n_surfaces - 1), GFP_ATOMIC);
+ if (ret == -EBUSY) {
qxl_reap_surface_id(qdev, 2);
goto again;
}
- surf->surface_id = handle;
+ if (ret < 0)
+ return ret;
- spin_lock(&qdev->surf_id_idr_lock);
- qdev->last_alloced_surf_id = handle;
- spin_unlock(&qdev->surf_id_idr_lock);
+ xa_lock(&qdev->surfaces);
+ qdev->last_alloced_surf_id = surf->surface_id;
+ xa_unlock(&qdev->surfaces);
return 0;
}
void qxl_surface_id_dealloc(struct qxl_device *qdev,
uint32_t surface_id)
{
- spin_lock(&qdev->surf_id_idr_lock);
- idr_remove(&qdev->surf_id_idr, surface_id);
- spin_unlock(&qdev->surf_id_idr_lock);
+ xa_erase(&qdev->surfaces, surface_id);
}
int qxl_hw_surface_alloc(struct qxl_device *qdev,
@@ -507,9 +493,7 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
qxl_release_fence_buffer_objects(release);
surf->hw_surf_alloc = true;
- spin_lock(&qdev->surf_id_idr_lock);
- idr_replace(&qdev->surf_id_idr, surf, surf->surface_id);
- spin_unlock(&qdev->surf_id_idr_lock);
+ xa_store(&qdev->surfaces, surf->surface_id, surf, GFP_KERNEL);
return 0;
}
@@ -531,10 +515,8 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
return ret;
surf->surf_create = NULL;
- /* remove the surface from the idr, but not the surface id yet */
- spin_lock(&qdev->surf_id_idr_lock);
- idr_replace(&qdev->surf_id_idr, NULL, surf->surface_id);
- spin_unlock(&qdev->surf_id_idr_lock);
+ /* remove the surface from the array, but don't free the surface id */
+ xa_store(&qdev->surfaces, surf->surface_id, NULL, 0);
surf->hw_surf_alloc = false;
id = surf->surface_id;
@@ -623,20 +605,20 @@ static int qxl_reap_surface_id(struct qxl_device *qdev,
int max_to_reap)
mutex_lock(&qdev->surf_evict_mutex);
again:
- spin_lock(&qdev->surf_id_idr_lock);
+ xa_lock(&qdev->surfaces);
start = qdev->last_alloced_surf_id + 1;
- spin_unlock(&qdev->surf_id_idr_lock);
+ xa_unlock(&qdev->surfaces);
for (i = start; i < start + qdev->rom->n_surfaces; i++) {
void *objptr;
int surfid = i % qdev->rom->n_surfaces;
- /* this avoids the case where the objects is in the
- idr but has been evicted half way - its makes
- the idr lookup atomic with the eviction */
- spin_lock(&qdev->surf_id_idr_lock);
- objptr = idr_find(&qdev->surf_id_idr, surfid);
- spin_unlock(&qdev->surf_id_idr_lock);
+ /* this avoids the case where the object is in the
+ array but has been evicted half way - it makes
+ the array lookup atomic with the eviction */
+ xa_lock(&qdev->surfaces);
+ objptr = xa_load(&qdev->surfaces, surfid);
+ xa_unlock(&qdev->surfaces);
if (!objptr)
continue;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 3abd432a4b85..8d0254a133dc 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -2
[PATCH 15/34] drm/amdkfd: Convert event_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 71 ++---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
2 files changed, 30 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index e9f0e0a1b41c..28adfb52d7ca 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -94,7 +94,7 @@ static struct kfd_signal_page *allocate_signal_page(struct
kfd_process *p)
static int allocate_event_notification_slot(struct kfd_process *p,
struct kfd_event *ev)
{
- int id;
+ int err;
if (!p->signal_page) {
p->signal_page = allocate_signal_page(p);
@@ -110,13 +110,12 @@ static int allocate_event_notification_slot(struct
kfd_process *p,
* KFD_SIGNAL_EVENT_LIMIT. This also allows future increase
* of the event limit without breaking user mode.
*/
- id = idr_alloc(&p->event_idr, ev, 0, p->signal_mapped_size / 8,
- GFP_KERNEL);
- if (id < 0)
- return id;
+ err = xa_alloc(&p->events, &ev->event_id, ev,
+ XA_LIMIT(0, p->signal_mapped_size / 8 - 1), GFP_KERNEL);
+ if (err < 0)
+ return err;
- ev->event_id = id;
- page_slots(p->signal_page)[id] = UNSIGNALED_EVENT_SLOT;
+ page_slots(p->signal_page)[ev->event_id] = UNSIGNALED_EVENT_SLOT;
return 0;
}
@@ -127,7 +126,7 @@ static int allocate_event_notification_slot(struct
kfd_process *p,
*/
static struct kfd_event *lookup_event_by_id(struct kfd_process *p, uint32_t id)
{
- return idr_find(&p->event_idr, id);
+ return xa_load(&p->events, id);
}
/**
@@ -162,7 +161,7 @@ static struct kfd_event
*lookup_signaled_event_by_partial_id(
if (page_slots(p->signal_page)[id] == UNSIGNALED_EVENT_SLOT)
return NULL;
- return idr_find(&p->event_idr, id);
+ return xa_load(&p->events, id);
}
/* General case for partial IDs: Iterate over all matching IDs
@@ -172,7 +171,7 @@ static struct kfd_event
*lookup_signaled_event_by_partial_id(
if (page_slots(p->signal_page)[id] == UNSIGNALED_EVENT_SLOT)
continue;
- ev = idr_find(&p->event_idr, id);
+ ev = xa_load(&p->events, id);
}
return ev;
@@ -211,26 +210,15 @@ static int create_signal_event(struct file *devkfd,
static int create_other_event(struct kfd_process *p, struct kfd_event *ev)
{
- /* Cast KFD_LAST_NONSIGNAL_EVENT to uint32_t. This allows an
-* intentional integer overflow to -1 without a compiler
-* warning. idr_alloc treats a negative value as "maximum
-* signed integer".
-*/
- int id = idr_alloc(&p->event_idr, ev, KFD_FIRST_NONSIGNAL_EVENT_ID,
- (uint32_t)KFD_LAST_NONSIGNAL_EVENT_ID + 1,
- GFP_KERNEL);
-
- if (id < 0)
- return id;
- ev->event_id = id;
-
- return 0;
+ return xa_alloc(&p->events, &ev->event_id, ev,
+ XA_LIMIT(KFD_FIRST_NONSIGNAL_EVENT_ID,
+ KFD_LAST_NONSIGNAL_EVENT_ID), GFP_KERNEL);
}
void kfd_event_init_process(struct kfd_process *p)
{
mutex_init(&p->event_mutex);
- idr_init(&p->event_idr);
+ xa_init_flags(&p->events, XA_FLAGS_ALLOC);
p->signal_page = NULL;
p->signal_event_count = 0;
}
@@ -248,18 +236,18 @@ static void destroy_event(struct kfd_process *p, struct
kfd_event *ev)
ev->type == KFD_EVENT_TYPE_DEBUG)
p->signal_event_count--;
- idr_remove(&p->event_idr, ev->event_id);
+ xa_erase(&p->events, ev->event_id);
kfree(ev);
}
static void destroy_events(struct kfd_process *p)
{
struct kfd_event *ev;
- uint32_t id;
+ unsigned long id;
- idr_for_each_entry(&p->event_idr, ev, id)
+ xa_for_each(&p->events, id, ev)
destroy_event(p, ev);
- idr_destroy(&p->event_idr);
+ xa_destroy(&p->events);
}
/*
@@ -490,7 +478,7 @@ void kfd_signal_event_interrupt(unsigned int pasid,
uint32_t partial_id,
* exhaustive search of signaled events.
*/
uint64_t *slots = page_slots(p->signal_page);
- uint32_t id;
+ unsigned long id;
if (valid_id_bits)
pr_debug_ratelimited("Partial ID invalid: %u (%u valid
bits)\n",
@@ -498,9 +486,9 @@ void kfd_signal_event_interrupt(unsigned
[PATCH 31/34] drm/vgem: Convert fence_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/vgem/vgem_drv.h | 3 +--
drivers/gpu/drm/vgem/vgem_fence.c | 43 +--
2 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
index 5c8f6d619ff3..56c8872b0967 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.h
+++ b/drivers/gpu/drm/vgem/vgem_drv.h
@@ -36,8 +36,7 @@
#include
struct vgem_file {
- struct idr fence_idr;
- struct mutex fence_mutex;
+ struct xarray fences;
};
#define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
b/drivers/gpu/drm/vgem/vgem_fence.c
index c1c420afe2dd..aa410ebcddf2 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -184,15 +184,10 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
reservation_object_add_shared_fence(resv, fence);
reservation_object_unlock(resv);
- /* Record the fence in our idr for later signaling */
+ /* Record the fence in our array for later signaling */
if (ret == 0) {
- mutex_lock(&vfile->fence_mutex);
- ret = idr_alloc(&vfile->fence_idr, fence, 1, 0, GFP_KERNEL);
- mutex_unlock(&vfile->fence_mutex);
- if (ret > 0) {
- arg->out_fence = ret;
- ret = 0;
- }
+ ret = xa_alloc(&vfile->fences, &arg->out_fence, fence,
+ xa_limit_31b, GFP_KERNEL);
}
err_fence:
if (ret) {
@@ -232,13 +227,13 @@ int vgem_fence_signal_ioctl(struct drm_device *dev,
if (arg->flags)
return -EINVAL;
- mutex_lock(&vfile->fence_mutex);
- fence = idr_replace(&vfile->fence_idr, NULL, arg->fence);
- mutex_unlock(&vfile->fence_mutex);
- if (!fence)
+ fence = xa_store(&vfile->fences, arg->fence, NULL, 0);
+ if (!fence) {
+ xa_erase(&vfile->fences, arg->fence);
+ return -ENOENT;
+ }
+ if (xa_is_err(fence))
return -ENOENT;
- if (IS_ERR(fence))
- return PTR_ERR(fence);
if (dma_fence_is_signaled(fence))
ret = -ETIMEDOUT;
@@ -250,21 +245,19 @@ int vgem_fence_signal_ioctl(struct drm_device *dev,
int vgem_fence_open(struct vgem_file *vfile)
{
- mutex_init(&vfile->fence_mutex);
- idr_init(&vfile->fence_idr);
+ xa_init_flags(&vfile->fences, XA_FLAGS_ALLOC1);
return 0;
}
-static int __vgem_fence_idr_fini(int id, void *p, void *data)
-{
- dma_fence_signal(p);
- dma_fence_put(p);
- return 0;
-}
-
void vgem_fence_close(struct vgem_file *vfile)
{
- idr_for_each(&vfile->fence_idr, __vgem_fence_idr_fini, vfile);
- idr_destroy(&vfile->fence_idr);
+ struct dma_fence *fence;
+ unsigned long index;
+
+ xa_for_each(&vfile->fences, index, fence) {
+ dma_fence_signal(fence);
+ dma_fence_put(fence);
+ }
+ xa_destroy(&vfile->fences);
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 24/34] drm/i915: Convert metrics_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_perf.c | 39
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1b7663258f42..5dd79453c525 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1805,7 +1805,7 @@ struct drm_i915_private {
/*
* Lock associated with adding/modifying/removing OA configs
-* in dev_priv->perf.metrics_idr.
+* in dev_priv->perf.configs.
*/
struct mutex metrics_lock;
@@ -1813,7 +1813,7 @@ struct drm_i915_private {
* List of dynamic configurations, you need to hold
* dev_priv->perf.metrics_lock to access it.
*/
- struct idr metrics_idr;
+ struct xarray configs;
/*
* Lock associated with anything below within this structure
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 2b2eb57ca71f..8a053772a11e 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -400,7 +400,7 @@ static int get_oa_config(struct drm_i915_private *dev_priv,
if (ret)
return ret;
- *out_config = idr_find(&dev_priv->perf.metrics_idr, metrics_set);
+ *out_config = xa_load(&dev_priv->perf.configs, metrics_set);
if (!*out_config)
ret = -EINVAL;
else
@@ -3142,7 +3142,8 @@ int i915_perf_add_config_ioctl(struct drm_device *dev,
void *data,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_perf_oa_config *args = data;
struct i915_oa_config *oa_config, *tmp;
- int err, id;
+ unsigned long id;
+ int err;
if (!dev_priv->perf.initialized) {
DRM_DEBUG("i915 perf interface not available for this
system\n");
@@ -3238,7 +3239,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev,
void *data,
/* We shouldn't have too many configs, so this iteration shouldn't be
* too costly.
*/
- idr_for_each_entry(&dev_priv->perf.metrics_idr, tmp, id) {
+ xa_for_each(&dev_priv->perf.configs, id, tmp) {
if (!strcmp(tmp->uuid, oa_config->uuid)) {
DRM_DEBUG("OA config already exists with this uuid\n");
err = -EADDRINUSE;
@@ -3253,12 +3254,10 @@ int i915_perf_add_config_ioctl(struct drm_device *dev,
void *data,
}
/* Config id 0 is invalid, id 1 for kernel stored test config. */
- oa_config->id = idr_alloc(&dev_priv->perf.metrics_idr,
- oa_config, 2,
- 0, GFP_KERNEL);
- if (oa_config->id < 0) {
+ err = xa_alloc(&dev_priv->perf.configs, &oa_config->id, oa_config,
+ XA_LIMIT(2, UINT_MAX), GFP_KERNEL);
+ if (err < 0) {
DRM_DEBUG("Failed to create sysfs entry for OA config\n");
- err = oa_config->id;
goto sysfs_err;
}
@@ -3309,7 +3308,7 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev,
void *data,
if (ret)
goto lock_err;
- oa_config = idr_find(&dev_priv->perf.metrics_idr, *arg);
+ oa_config = xa_erase(&dev_priv->perf.configs, *arg);
if (!oa_config) {
DRM_DEBUG("Failed to remove unknown OA config\n");
ret = -ENOENT;
@@ -3321,8 +3320,6 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev,
void *data,
sysfs_remove_group(dev_priv->perf.metrics_kobj,
&oa_config->sysfs_metric);
- idr_remove(&dev_priv->perf.metrics_idr, *arg);
-
DRM_DEBUG("Removed config %s id=%i\n", oa_config->uuid, oa_config->id);
put_oa_config(dev_priv, oa_config);
@@ -3475,33 +3472,27 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
dev_priv->perf.sysctl_header = register_sysctl_table(dev_root);
mutex_init(&dev_priv->perf.metrics_lock);
- idr_init(&dev_priv->perf.metrics_idr);
+ xa_init_flags(&dev_priv->perf.configs, XA_FLAGS_ALLOC);
dev_priv->perf.initialized = true;
}
}
-static int destroy_config(int id, void *p, void *data)
-{
- struct drm_i915_private *dev_priv = data;
- struct i915_oa_config *oa_config = p;
-
- put_oa_config(dev_priv, oa_config);
-
- return 0;
-}
-
/**
* i915_perf_fini - Counter part to i915_perf_init()
* @dev_priv: i915 device instance
*/
void i915
[PATCH 06/34] drm: Convert magic_map to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_auth.c | 18 --
include/drm/drm_auth.h | 5 ++---
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1669c42c40ed..268189e9e2e0 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -62,17 +62,16 @@ int drm_getmagic(struct drm_device *dev, void *data, struct
drm_file *file_priv)
mutex_lock(&dev->master_mutex);
if (!file_priv->magic) {
- ret = idr_alloc(&file_priv->master->magic_map, file_priv,
- 1, 0, GFP_KERNEL);
- if (ret >= 0)
- file_priv->magic = ret;
+ ret = xa_alloc(&file_priv->master->magic_map,
+ &file_priv->magic, file_priv,
+ xa_limit_31b, GFP_KERNEL);
}
auth->magic = file_priv->magic;
mutex_unlock(&dev->master_mutex);
DRM_DEBUG("%u\n", auth->magic);
- return ret < 0 ? ret : 0;
+ return ret;
}
int drm_authmagic(struct drm_device *dev, void *data,
@@ -84,10 +83,10 @@ int drm_authmagic(struct drm_device *dev, void *data,
DRM_DEBUG("%u\n", auth->magic);
mutex_lock(&dev->master_mutex);
- file = idr_find(&file_priv->master->magic_map, auth->magic);
+ file = xa_load(&file_priv->master->magic_map, auth->magic);
if (file) {
file->authenticated = 1;
- idr_replace(&file_priv->master->magic_map, NULL, auth->magic);
+ xa_store(&file_priv->master->magic_map, auth->magic, NULL, 0);
}
mutex_unlock(&dev->master_mutex);
@@ -105,7 +104,7 @@ struct drm_master *drm_master_create(struct drm_device *dev)
kref_init(&master->refcount);
spin_lock_init(&master->lock.spinlock);
init_waitqueue_head(&master->lock.lock_queue);
- idr_init(&master->magic_map);
+ xa_init_flags(&master->magic_map, XA_FLAGS_ALLOC1);
master->dev = dev;
/* initialize the tree of output resource lessees */
@@ -269,7 +268,7 @@ void drm_master_release(struct drm_file *file_priv)
mutex_lock(&dev->master_mutex);
if (file_priv->magic)
- idr_remove(&file_priv->master->magic_map, file_priv->magic);
+ xa_erase(&file_priv->master->magic_map, file_priv->magic);
if (!drm_is_current_master(file_priv))
goto out;
@@ -348,7 +347,6 @@ static void drm_master_destroy(struct kref *kref)
drm_legacy_master_rmmaps(dev, master);
- idr_destroy(&master->magic_map);
idr_destroy(&master->leases);
idr_destroy(&master->lessee_idr);
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 86bff9841b54..c719fe375967 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -76,10 +76,9 @@ struct drm_master {
*/
int unique_len;
/**
-* @magic_map: Map of used authentication tokens. Protected by
-* &drm_device.master_mutex.
+* @magic_map: Map of used authentication tokens.
*/
- struct idr magic_map;
+ struct xarray magic_map;
struct drm_lock_data lock;
void *driver_priv;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/34] drm: Convert lessee_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_auth.c | 3 +--
drivers/gpu/drm/drm_lease.c | 15 ++-
include/drm/drm_auth.h | 2 +-
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 268189e9e2e0..28767f55b30b 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -113,7 +113,7 @@ struct drm_master *drm_master_create(struct drm_device *dev)
INIT_LIST_HEAD(&master->lessees);
INIT_LIST_HEAD(&master->lessee_list);
idr_init(&master->leases);
- idr_init(&master->lessee_idr);
+ xa_init_flags(&master->lessee_xa, XA_FLAGS_ALLOC1);
return master;
}
@@ -348,7 +348,6 @@ static void drm_master_destroy(struct kref *kref)
drm_legacy_master_rmmaps(dev, master);
idr_destroy(&master->leases);
- idr_destroy(&master->lessee_idr);
kfree(master->unique);
kfree(master);
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 99cba8ea5d82..c02587443b61 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -54,7 +54,7 @@ static struct drm_master*
_drm_find_lessee(struct drm_master *master, int lessee_id)
{
lockdep_assert_held(&master->dev->mode_config.idr_mutex);
- return idr_find(&drm_lease_owner(master)->lessee_idr, lessee_id);
+ return xa_load(&drm_lease_owner(master)->lessee_xa, lessee_id);
}
/**
@@ -203,7 +203,6 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
int error;
struct drm_master *lessee;
int object;
- int id;
void *entry;
DRM_DEBUG_LEASE("lessor %d\n", lessor->lessee_id);
@@ -232,13 +231,11 @@ static struct drm_master *drm_lease_create(struct
drm_master *lessor, struct idr
}
/* Insert the new lessee into the tree */
- id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0,
GFP_KERNEL);
- if (id < 0) {
- error = id;
+ error = xa_alloc(&drm_lease_owner(lessor)->lessee_xa,
+ &lessee->lessee_id, lessee, xa_limit_32b, GFP_KERNEL);
+ if (error < 0)
goto out_lessee;
- }
- lessee->lessee_id = id;
lessee->lessor = drm_master_get(lessor);
list_add_tail(&lessee->lessee_list, &lessor->lessees);
@@ -279,10 +276,10 @@ void drm_lease_destroy(struct drm_master *master)
*/
WARN_ON(!list_empty(&master->lessees));
- /* Remove this master from the lessee idr in the owner */
+ /* Remove this master from the lessee array in the owner */
if (master->lessee_id != 0) {
DRM_DEBUG_LEASE("remove master %d from device list of
lessees\n", master->lessee_id);
- idr_remove(&(drm_lease_owner(master)->lessee_idr),
master->lessee_id);
+ xa_erase(&drm_lease_owner(master)->lessee_xa,
master->lessee_id);
}
/* Remove this master from any lessee list it may be on */
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index c719fe375967..f1e092406caa 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -93,7 +93,7 @@ struct drm_master {
struct list_head lessee_list;
struct list_head lessees;
struct idr leases;
- struct idr lessee_idr;
+ struct xarray lessee_xa;
};
struct drm_master *drm_master_get(struct drm_master *master);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 26/34] drm/qxl: Convert release_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/qxl/qxl_drv.h | 3 +-
drivers/gpu/drm/qxl/qxl_kms.c | 3 +-
drivers/gpu/drm/qxl/qxl_release.c | 54 +--
3 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 13a0254b59a1..3abd432a4b85 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -238,9 +238,8 @@ struct qxl_device {
uint64_tva_slot_mask;
spinlock_t release_lock;
- struct idr release_idr;
+ struct xarray releases;
uint32_trelease_seqno;
- spinlock_t release_idr_lock;
struct mutexasync_io_mutex;
unsigned int last_sent_io_cmd;
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 15238a413f9d..b2cc71c95142 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -247,8 +247,7 @@ int qxl_device_init(struct qxl_device *qdev,
goto release_ring_free;
}
- idr_init(&qdev->release_idr);
- spin_lock_init(&qdev->release_idr_lock);
+ xa_init_flags(&qdev->releases, XA_FLAGS_ALLOC1);
spin_lock_init(&qdev->release_lock);
idr_init(&qdev->surf_id_idr);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 30f85f0130cb..a1a42662ebf4 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -121,7 +121,7 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
struct qxl_release **ret)
{
struct qxl_release *release;
- int handle;
+ int err;
size_t size = sizeof(*release);
release = kmalloc(size, GFP_KERNEL);
@@ -135,21 +135,19 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
release->surface_release_id = 0;
INIT_LIST_HEAD(&release->bos);
- idr_preload(GFP_KERNEL);
- spin_lock(&qdev->release_idr_lock);
- handle = idr_alloc(&qdev->release_idr, release, 1, 0, GFP_NOWAIT);
+ xa_lock(&qdev->releases);
+ err = __xa_alloc(&qdev->releases, &release->id, release, xa_limit_31b,
+ GFP_KERNEL);
release->base.seqno = ++qdev->release_seqno;
- spin_unlock(&qdev->release_idr_lock);
- idr_preload_end();
- if (handle < 0) {
+ xa_unlock(&qdev->releases);
+ if (err < 0) {
kfree(release);
*ret = NULL;
- return handle;
+ return err;
}
*ret = release;
- DRM_DEBUG_DRIVER("allocated release %d\n", handle);
- release->id = handle;
- return handle;
+ DRM_DEBUG_DRIVER("allocated release %d\n", release->id);
+ return release->id;
}
static void
@@ -178,9 +176,7 @@ qxl_release_free(struct qxl_device *qdev,
if (release->surface_release_id)
qxl_surface_id_dealloc(qdev, release->surface_release_id);
- spin_lock(&qdev->release_idr_lock);
- idr_remove(&qdev->release_idr, release->id);
- spin_unlock(&qdev->release_idr_lock);
+ xa_erase(&qdev->releases, release->id);
if (release->base.ops) {
WARN_ON(list_empty(&release->bos));
@@ -288,14 +284,14 @@ int qxl_alloc_surface_release_reserved(struct qxl_device
*qdev,
struct qxl_release **release)
{
if (surface_cmd_type == QXL_SURFACE_CMD_DESTROY && create_rel) {
- int idr_ret;
+ int id;
struct qxl_bo *bo;
union qxl_release_info *info;
/* stash the release after the create command */
- idr_ret = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD,
release);
- if (idr_ret < 0)
- return idr_ret;
+ id = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD, release);
+ if (id < 0)
+ return id;
bo = create_rel->release_bo;
(*release)->release_bo = bo;
@@ -304,7 +300,7 @@ int qxl_alloc_surface_release_reserved(struct qxl_device
*qdev,
qxl_release_list_add(*release, bo);
info = qxl_release_map(qdev, *release);
- info->id = idr_ret;
+ info->id = id;
qxl_release_unmap(qdev, *release, info);
return 0;
}
@@ -318,7 +314,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
struct qxl_bo **rbo)
{
struct qxl_bo *bo;
- int idr_ret;
+ int id;
int ret = 0;
union qxl_release_info *info;
[PATCH 22/34] drm/i915: Convert object_idr to IDA
I suspect dmabuf_obj_list_head and object_ids should be combined into
a single xarray, but that's a job for later.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/gvt/dmabuf.c | 7 +++
drivers/gpu/drm/i915/gvt/gvt.h| 2 +-
drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
b/drivers/gpu/drm/i915/gvt/dmabuf.c
index 51ed99a37803..9fb60461affc 100644
--- a/drivers/gpu/drm/i915/gvt/dmabuf.c
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -96,8 +96,7 @@ static void dmabuf_gem_object_free(struct kref *kref)
struct intel_vgpu_dmabuf_obj, list);
if (dmabuf_obj == obj) {
intel_gvt_hypervisor_put_vfio_device(vgpu);
- idr_remove(&vgpu->object_idr,
- dmabuf_obj->dmabuf_id);
+ ida_free(&vgpu->object_ids,
dmabuf_obj->dmabuf_id);
kfree(dmabuf_obj->info);
kfree(dmabuf_obj);
list_del(pos);
@@ -431,7 +430,7 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void
*args)
dmabuf_obj->vgpu = vgpu;
- ret = idr_alloc(&vgpu->object_idr, dmabuf_obj, 1, 0, GFP_NOWAIT);
+ ret = ida_alloc_min(&vgpu->object_ids, 1, GFP_NOWAIT);
if (ret < 0)
goto out_free_info;
gfx_plane_info->dmabuf_id = ret;
@@ -553,7 +552,7 @@ void intel_vgpu_dmabuf_cleanup(struct intel_vgpu *vgpu)
list);
dmabuf_obj->vgpu = NULL;
- idr_remove(&vgpu->object_idr, dmabuf_obj->dmabuf_id);
+ ida_free(&vgpu->object_ids, dmabuf_obj->dmabuf_id);
intel_gvt_hypervisor_put_vfio_device(vgpu);
list_del(pos);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index e5bf20dcdd7d..ffb181a086be 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -231,7 +231,7 @@ struct intel_vgpu {
struct list_head dmabuf_obj_list_head;
struct mutex dmabuf_lock;
- struct idr object_idr;
+ struct ida object_ids;
struct completion vblank_done;
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 6ec5d16f4e06..c1db9a6a1281 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -383,7 +383,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct
intel_gvt *gvt,
mutex_init(&vgpu->dmabuf_lock);
INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head);
xa_init(&vgpu->page_track);
- idr_init(&vgpu->object_idr);
+ ida_init(&vgpu->object_ids);
intel_vgpu_init_cfg_space(vgpu, param->primary);
ret = intel_vgpu_init_mmio(vgpu);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/34] drm: Convert tile_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/drm_connector.c | 27 +++
drivers/gpu/drm/drm_mode_config.c | 3 +--
include/drm/drm_mode_config.h | 12 ++--
3 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da8ae80c2750..682eb79b721a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2019,9 +2019,7 @@ static void drm_tile_group_free(struct kref *kref)
{
struct drm_tile_group *tg = container_of(kref, struct drm_tile_group,
refcount);
struct drm_device *dev = tg->dev;
- mutex_lock(&dev->mode_config.idr_mutex);
- idr_remove(&dev->mode_config.tile_idr, tg->id);
- mutex_unlock(&dev->mode_config.idr_mutex);
+ xa_erase(&dev->mode_config.tiles, tg->id);
kfree(tg);
}
@@ -2053,18 +2051,18 @@ struct drm_tile_group *drm_mode_get_tile_group(struct
drm_device *dev,
char topology[8])
{
struct drm_tile_group *tg;
- int id;
- mutex_lock(&dev->mode_config.idr_mutex);
- idr_for_each_entry(&dev->mode_config.tile_idr, tg, id) {
+ unsigned long id;
+
+ xa_lock(&dev->mode_config.tiles);
+ xa_for_each(&dev->mode_config.tiles, id, tg) {
if (!memcmp(tg->group_data, topology, 8)) {
if (!kref_get_unless_zero(&tg->refcount))
tg = NULL;
- mutex_unlock(&dev->mode_config.idr_mutex);
- return tg;
+ break;
}
}
- mutex_unlock(&dev->mode_config.idr_mutex);
- return NULL;
+ xa_unlock(&dev->mode_config.tiles);
+ return tg;
}
EXPORT_SYMBOL(drm_mode_get_tile_group);
@@ -2093,16 +2091,13 @@ struct drm_tile_group
*drm_mode_create_tile_group(struct drm_device *dev,
memcpy(tg->group_data, topology, 8);
tg->dev = dev;
- mutex_lock(&dev->mode_config.idr_mutex);
- ret = idr_alloc(&dev->mode_config.tile_idr, tg, 1, 0, GFP_KERNEL);
- if (ret >= 0) {
- tg->id = ret;
- } else {
+ ret = xa_alloc(&dev->mode_config.tiles, &tg->id, tg, xa_limit_32b,
+ GFP_KERNEL);
+ if (ret < 0) {
kfree(tg);
tg = ERR_PTR(ret);
}
- mutex_unlock(&dev->mode_config.idr_mutex);
return tg;
}
EXPORT_SYMBOL(drm_mode_create_tile_group);
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index 703bfce975bb..609b30d7dcb1 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -394,7 +394,7 @@ void drm_mode_config_init(struct drm_device *dev)
INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
INIT_LIST_HEAD(&dev->mode_config.plane_list);
idr_init(&dev->mode_config.crtc_idr);
- idr_init(&dev->mode_config.tile_idr);
+ xa_init_flags(&dev->mode_config.tiles, XA_FLAGS_ALLOC1);
ida_init(&dev->mode_config.connector_ida);
spin_lock_init(&dev->mode_config.connector_list_lock);
@@ -495,7 +495,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
}
ida_destroy(&dev->mode_config.connector_ida);
- idr_destroy(&dev->mode_config.tile_idr);
idr_destroy(&dev->mode_config.crtc_idr);
drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
}
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 572274ccbec7..fea334d99201 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -391,8 +391,8 @@ struct drm_mode_config {
/**
* @idr_mutex:
*
-* Mutex for KMS ID allocation and management. Protects both @crtc_idr
-* and @tile_idr.
+* Mutex for KMS ID allocation and management. Protects the
+* objects in @objects.
*/
struct mutex idr_mutex;
@@ -405,12 +405,12 @@ struct drm_mode_config {
struct idr crtc_idr;
/**
-* @tile_idr:
+* @tiles:
*
-* Use this idr for allocating new IDs for tiled sinks like use in some
-* high-res DP MST screens.
+* Use this for allocating new IDs for tiled sinks like those
+* used in some high-res DP MST screens.
*/
- struct idr tile_idr;
+ struct xarray tiles;
/** @fb_lock: Mutex to protect fb the global @fb_list and @num_fb. */
struct mutex fb_lock;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 13/34] drm/amdgpu: Convert ctx_handles to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 42 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 4 +--
4 files changed, 19 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6a704aaa7dbe..c2650f143ba7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -164,7 +164,7 @@ extern int amdgpu_si_support;
extern int amdgpu_cik_support;
#endif
-#define AMDGPU_VM_MAX_NUM_CTX 4096
+#define AMDGPU_VM_CTX_LIMITXA_LIMIT(0, 4095)
#define AMDGPU_SG_THRESHOLD(256*1024*1024)
#define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
#define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index d85184b5b35c..bddc28b1c9ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -248,17 +248,17 @@ static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
return -ENOMEM;
mutex_lock(&mgr->lock);
- r = idr_alloc(&mgr->ctx_handles, ctx, 1, AMDGPU_VM_MAX_NUM_CTX,
GFP_KERNEL);
+ r = xa_alloc(&mgr->ctx_handles, id, ctx, AMDGPU_VM_CTX_LIMIT,
+ GFP_KERNEL);
if (r < 0) {
mutex_unlock(&mgr->lock);
kfree(ctx);
return r;
}
- *id = (uint32_t)r;
r = amdgpu_ctx_init(adev, priority, filp, ctx);
if (r) {
- idr_remove(&mgr->ctx_handles, *id);
+ xa_erase(&mgr->ctx_handles, *id);
*id = 0;
kfree(ctx);
}
@@ -290,7 +290,7 @@ static int amdgpu_ctx_free(struct amdgpu_fpriv *fpriv,
uint32_t id)
struct amdgpu_ctx *ctx;
mutex_lock(&mgr->lock);
- ctx = idr_remove(&mgr->ctx_handles, id);
+ ctx = xa_erase(&mgr->ctx_handles, id);
if (ctx)
kref_put(&ctx->refcount, amdgpu_ctx_do_release);
mutex_unlock(&mgr->lock);
@@ -310,7 +310,7 @@ static int amdgpu_ctx_query(struct amdgpu_device *adev,
mgr = &fpriv->ctx_mgr;
mutex_lock(&mgr->lock);
- ctx = idr_find(&mgr->ctx_handles, id);
+ ctx = xa_load(&mgr->ctx_handles, id);
if (!ctx) {
mutex_unlock(&mgr->lock);
return -EINVAL;
@@ -345,7 +345,7 @@ static int amdgpu_ctx_query2(struct amdgpu_device *adev,
mgr = &fpriv->ctx_mgr;
mutex_lock(&mgr->lock);
- ctx = idr_find(&mgr->ctx_handles, id);
+ ctx = xa_load(&mgr->ctx_handles, id);
if (!ctx) {
mutex_unlock(&mgr->lock);
return -EINVAL;
@@ -419,7 +419,7 @@ struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv
*fpriv, uint32_t id)
mgr = &fpriv->ctx_mgr;
mutex_lock(&mgr->lock);
- ctx = idr_find(&mgr->ctx_handles, id);
+ ctx = xa_load(&mgr->ctx_handles, id);
if (ctx)
kref_get(&ctx->refcount);
mutex_unlock(&mgr->lock);
@@ -533,22 +533,18 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
{
mutex_init(&mgr->lock);
- idr_init(&mgr->ctx_handles);
+ xa_init_flags(&mgr->ctx_handles, XA_FLAGS_ALLOC1);
}
void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr)
{
unsigned num_entities = amdgput_ctx_total_num_entities();
struct amdgpu_ctx *ctx;
- struct idr *idp;
- uint32_t id, i;
+ unsigned long id, i;
long max_wait = MAX_WAIT_SCHED_ENTITY_Q_EMPTY;
- idp = &mgr->ctx_handles;
-
mutex_lock(&mgr->lock);
- idr_for_each_entry(idp, ctx, id) {
-
+ xa_for_each(&mgr->ctx_handles, id, ctx) {
if (!ctx->adev) {
mutex_unlock(&mgr->lock);
return;
@@ -568,13 +564,9 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr)
{
unsigned num_entities = amdgput_ctx_total_num_entities();
struct amdgpu_ctx *ctx;
- struct idr *idp;
- uint32_t id, i;
-
- idp = &mgr->ctx_handles;
-
- idr_for_each_entry(idp, ctx, id) {
+ unsigned long id, i;
+ xa_for_each(&mgr->ctx_handles, id, ctx) {
if (!ctx->adev)
return;
@@ -591,18 +583,14 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr
*mgr)
void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
{
struct amdgpu_ctx *ctx;
-
[PATCH 18/34] drm/i915: Convert handles_vma to XArray
Straightforward conversion.
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 12 +---
drivers/gpu/drm/i915/i915_gem_context.h | 4 ++--
drivers/gpu/drm/i915/i915_gem_execbuffer.c| 6 +++---
drivers/gpu/drm/i915/selftests/mock_context.c | 2 +-
5 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 216f52b744a6..0cdccc886587 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3663,7 +3663,7 @@ void i915_gem_close_object(struct drm_gem_object *gem,
struct drm_file *file)
if (ctx->file_priv != fpriv)
continue;
- vma = radix_tree_delete(&ctx->handles_vma, lut->handle);
+ vma = xa_erase(&ctx->handles_vma, lut->handle);
GEM_BUG_ON(vma->obj != obj);
/* We allow the process to have multiple handles to the same
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 371c07087095..9db04b2e65cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -96,9 +96,9 @@
static void lut_close(struct i915_gem_context *ctx)
{
+ XA_STATE(xas, &ctx->handles_vma, 0);
struct i915_lut_handle *lut, *ln;
- struct radix_tree_iter iter;
- void __rcu **slot;
+ struct i915_vma *vma;
list_for_each_entry_safe(lut, ln, &ctx->handles_list, ctx_link) {
list_del(&lut->obj_link);
@@ -106,10 +106,8 @@ static void lut_close(struct i915_gem_context *ctx)
}
rcu_read_lock();
- radix_tree_for_each_slot(slot, &ctx->handles_vma, &iter, 0) {
- struct i915_vma *vma = rcu_dereference_raw(*slot);
-
- radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
+ xas_for_each(&xas, vma, ULONG_MAX) {
+ xas_store(&xas, NULL);
__i915_gem_object_release_unless_active(vma->obj);
}
rcu_read_unlock();
@@ -345,7 +343,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
ce->gem_context = ctx;
}
- INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
+ xa_init(&ctx->handles_vma);
INIT_LIST_HEAD(&ctx->handles_list);
INIT_LIST_HEAD(&ctx->hw_id_link);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h
b/drivers/gpu/drm/i915/i915_gem_context.h
index f6d870b1f73e..ec22de370a22 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -194,11 +194,11 @@ struct i915_gem_context {
/** remap_slice: Bitmask of cache lines that need remapping */
u8 remap_slice;
- /** handles_vma: rbtree to look up our context specific obj/vma for
+ /** handles_vma: lookup our context specific obj/vma for
* the user handle. (user handles are per fd, but the binding is
* per vm, which may be one per context or shared with the global GTT)
*/
- struct radix_tree_root handles_vma;
+ struct xarray handles_vma;
/** handles_list: reverse list of all the rbtree entries in use for
* this context, which allows us to free all the allocations on
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 485b259127c3..8f136da3f73a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -756,7 +756,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
static int eb_lookup_vmas(struct i915_execbuffer *eb)
{
- struct radix_tree_root *handles_vma = &eb->ctx->handles_vma;
+ struct xarray *handles_vma = &eb->ctx->handles_vma;
struct drm_i915_gem_object *obj;
unsigned int i, batch;
int err;
@@ -777,7 +777,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
struct i915_lut_handle *lut;
struct i915_vma *vma;
- vma = radix_tree_lookup(handles_vma, handle);
+ vma = xa_load(handles_vma, handle);
if (likely(vma))
goto add_vma;
@@ -799,7 +799,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
goto err_obj;
}
- err = radix_tree_insert(handles_vma, handle, vma);
+ err = xa_err(xa_store(handles_vma, handle, vma, GFP_KERNEL));
if (unlikely(err)) {
kmem_cache_free(eb->i915->luts, lut);
goto err_obj;
diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c
b/drivers/gpu/drm/i915/selftests/mock_context.c
index d937bdff26f9..a
[PATCH 23/34] drm/i915: Convert context_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/i915/i915_debugfs.c | 32 -
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 31 ++--
3 files changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 030263870ba6..4981d1f6e7a5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -376,9 +376,9 @@ static void print_batch_pool_stats(struct seq_file *m,
print_file_stats(m, "[k]batch pool", stats);
}
-static int per_file_ctx_stats(int idx, void *ptr, void *data)
+static void per_file_ctx_stats(struct i915_gem_context *ctx,
+ struct file_stats *stats)
{
- struct i915_gem_context *ctx = ptr;
struct intel_engine_cs *engine;
enum intel_engine_id id;
@@ -386,12 +386,10 @@ static int per_file_ctx_stats(int idx, void *ptr, void
*data)
struct intel_context *ce = to_intel_context(ctx, engine);
if (ce->state)
- per_file_stats(ce->state->obj, data);
+ per_file_stats(ce->state->obj, stats);
if (ce->ring)
- per_file_stats(ce->ring->vma->obj, data);
+ per_file_stats(ce->ring->vma->obj, stats);
}
-
- return 0;
}
static void print_context_stats(struct seq_file *m,
@@ -405,11 +403,15 @@ static void print_context_stats(struct seq_file *m,
mutex_lock(&dev->struct_mutex);
if (dev_priv->kernel_context)
- per_file_ctx_stats(0, dev_priv->kernel_context, &stats);
+ per_file_ctx_stats(dev_priv->kernel_context, &stats);
list_for_each_entry(file, &dev->filelist, lhead) {
struct drm_i915_file_private *fpriv = file->driver_priv;
- idr_for_each(&fpriv->context_idr, per_file_ctx_stats, &stats);
+ struct i915_gem_context *ctx;
+ unsigned long index;
+
+ xa_for_each(&fpriv->contexts, index, ctx)
+ per_file_ctx_stats(ctx, &stats);
}
mutex_unlock(&dev->struct_mutex);
@@ -2078,16 +2080,14 @@ static int i915_swizzle_info(struct seq_file *m, void
*data)
return 0;
}
-static int per_file_ctx(int id, void *ptr, void *data)
+static void per_file_ctx(struct i915_gem_context *ctx, struct seq_file *m)
{
- struct i915_gem_context *ctx = ptr;
- struct seq_file *m = data;
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
if (!ppgtt) {
seq_printf(m, " no ppgtt for context %d\n",
ctx->user_handle);
- return 0;
+ return;
}
if (i915_gem_context_is_default(ctx))
@@ -2095,8 +2095,6 @@ static int per_file_ctx(int id, void *ptr, void *data)
else
seq_printf(m, " context %d:\n", ctx->user_handle);
ppgtt->debug_dump(ppgtt, m);
-
- return 0;
}
static void gen8_ppgtt_info(struct seq_file *m,
@@ -2176,6 +2174,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
list_for_each_entry_reverse(file, &dev->filelist, lhead) {
struct drm_i915_file_private *file_priv = file->driver_priv;
struct task_struct *task;
+ struct i915_gem_context *ctx;
+ unsigned long index;
task = get_pid_task(file->pid, PIDTYPE_PID);
if (!task) {
@@ -2184,8 +2184,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
}
seq_printf(m, "\nproc: %s\n", task->comm);
put_task_struct(task);
- idr_for_each(&file_priv->context_idr, per_file_ctx,
-(void *)(unsigned long)m);
+ xa_for_each(&file_priv->contexts, index, ctx)
+ per_file_ctx(ctx, m);
}
out_rpm:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4b5ce517cbcf..1b7663258f42 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -213,7 +213,7 @@ struct drm_i915_file_private {
*/
#define DRM_I915_THROTTLE_JIFFIES msecs_to_jiffies(20)
} mm;
- struct idr context_idr;
+ struct xarray contexts;
struct intel_rps_client {
atomic_t boosts;
@@ -3118,7 +3118,7 @@ void i915_gem_object_save_bit_17_swizzle(struct
drm_i915_gem_object *obj,
static inline struct i915_gem_context *
__i915_gem_context_lookup_rcu(struct drm_i915_file_private *file_priv, u32 id)
{
- return idr_find(&file_priv->context_idr, id);
+ return xa_load(&file_priv->contexts, id);
[PATCH 32/34] drm/via: Convert object_idr to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/via/via_drv.h | 2 +-
drivers/gpu/drm/via/via_map.c | 4 +---
drivers/gpu/drm/via/via_mm.c | 11 +--
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h
index 6d1ae834484c..df0f451908e2 100644
--- a/drivers/gpu/drm/via/via_drv.h
+++ b/drivers/gpu/drm/via/via_drv.h
@@ -95,7 +95,7 @@ typedef struct drm_via_private {
int agp_initialized;
struct drm_mm agp_mm;
/** Mapping of userspace keys to mm objects */
- struct idr object_idr;
+ struct xarray objects;
unsigned long vram_offset;
unsigned long agp_offset;
drm_via_blitq_t blit_queues[VIA_NUM_BLIT_ENGINES];
diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
index 2ad865870372..c47630105f5b 100644
--- a/drivers/gpu/drm/via/via_map.c
+++ b/drivers/gpu/drm/via/via_map.c
@@ -100,7 +100,7 @@ int via_driver_load(struct drm_device *dev, unsigned long
chipset)
if (dev_priv == NULL)
return -ENOMEM;
- idr_init(&dev_priv->object_idr);
+ xa_init_flags(&dev_priv->objects, XA_FLAGS_ALLOC1);
dev->dev_private = (void *)dev_priv;
dev_priv->chipset = chipset;
@@ -120,7 +120,5 @@ void via_driver_unload(struct drm_device *dev)
{
drm_via_private_t *dev_priv = dev->dev_private;
- idr_destroy(&dev_priv->object_idr);
-
kfree(dev_priv);
}
diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c
index 4217d66a5cc6..aaa90a096e52 100644
--- a/drivers/gpu/drm/via/via_mm.c
+++ b/drivers/gpu/drm/via/via_mm.c
@@ -148,10 +148,10 @@ int via_mem_alloc(struct drm_device *dev, void *data,
if (retval)
goto fail_alloc;
- retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL);
+ retval = xa_alloc(&dev_priv->objects, &user_key, item,
+ xa_limit_31b, GFP_KERNEL);
if (retval < 0)
- goto fail_idr;
- user_key = retval;
+ goto fail_xa;
list_add(&item->owner_list, &file_priv->obj_list);
mutex_unlock(&dev->struct_mutex);
@@ -163,7 +163,7 @@ int via_mem_alloc(struct drm_device *dev, void *data,
return 0;
-fail_idr:
+fail_xa:
drm_mm_remove_node(&item->mm_node);
fail_alloc:
kfree(item);
@@ -184,13 +184,12 @@ int via_mem_free(struct drm_device *dev, void *data,
struct drm_file *file_priv)
struct via_memblock *obj;
mutex_lock(&dev->struct_mutex);
- obj = idr_find(&dev_priv->object_idr, mem->index);
+ obj = xa_erase(&dev_priv->objects, mem->index);
if (obj == NULL) {
mutex_unlock(&dev->struct_mutex);
return -EINVAL;
}
- idr_remove(&dev_priv->object_idr, mem->index);
list_del(&obj->owner_list);
drm_mm_remove_node(&obj->mm_node);
kfree(obj);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/34] drm: Convert crtc_idr to XArray
- Rename it to 'objects', as requested in todo.rst
- Also convert leases IDR to XArray as the two are occasionally used by
the same code (see drm_mode_get_lease_ioctl())
- Refactor drm_mode_create_lease_ioctl() to create the new drm_master
early to avoid creating an XArray on the stack and reparenting it
afterwards.
Signed-off-by: Matthew Wilcox
---
Documentation/gpu/todo.rst| 3 -
drivers/gpu/drm/drm_auth.c| 4 +-
drivers/gpu/drm/drm_lease.c | 136 ++
drivers/gpu/drm/drm_mode_config.c | 3 +-
drivers/gpu/drm/drm_mode_object.c | 47 +--
include/drm/drm_auth.h| 2 +-
include/drm/drm_mode_config.h | 6 +-
7 files changed, 90 insertions(+), 111 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 14191b64446d..41da7b06195c 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -354,9 +354,6 @@ KMS cleanups
Some of these date from the very introduction of KMS in 2008 ...
-- drm_mode_config.crtc_idr is misnamed, since it contains all KMS object.
Should
- be renamed to drm_mode_config.object_idr.
-
- drm_display_mode doesn't need to be derived from drm_mode_object. That's
leftovers from older (never merged into upstream) KMS designs where modes
where set using their ID, including support to add/remove modes.
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1813507f9b9c..c6967f0b095d 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -110,7 +110,7 @@ struct drm_master *drm_master_create(struct drm_device *dev)
/* initialize the tree of output resource lessees */
master->lessor = NULL;
master->lessee_id = 0;
- idr_init(&master->leases);
+ xa_init(&master->leases);
xa_init_flags(&master->lessees, XA_FLAGS_ALLOC1);
return master;
@@ -345,7 +345,7 @@ static void drm_master_destroy(struct kref *kref)
drm_legacy_master_rmmaps(dev, master);
- idr_destroy(&master->leases);
+ xa_destroy(&master->leases);
kfree(master->unique);
kfree(master);
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 47830f9ec616..1e88f406c738 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -20,8 +20,6 @@
#include
#include
-static uint64_t drm_lease_idr_object;
-
/**
* drm_lease_owner - return ancestor owner drm_master
* @master: drm_master somewhere within tree of lessees and lessors
@@ -69,7 +67,7 @@ static int _drm_lease_held_master(struct drm_master *master,
int id)
{
lockdep_assert_held(&master->dev->mode_config.idr_mutex);
if (master->lessor)
- return idr_find(&master->leases, id) != NULL;
+ return xa_load(&master->leases, id) != NULL;
return true;
}
@@ -183,7 +181,7 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv,
uint32_t crtcs_in)
/*
* drm_lease_create - create a new drm_master with leased objects (idr_mutex
not held)
* @lessor: lease holder (or owner) of objects
- * @leases: objects to lease to the new drm_master
+ * @lessee: leaser of objects
*
* Uses drm_master_create to allocate a new drm_master, then checks to
* make sure all of the desired objects can be leased, atomically
@@ -195,35 +193,30 @@ uint32_t drm_lease_filter_crtcs(struct drm_file
*file_priv, uint32_t crtcs_in)
* ERR_PTR(-EEXIST)same object specified more than once in the
provided list
* ERR_PTR(-ENOMEM)allocation failed
*/
-static struct drm_master *drm_lease_create(struct drm_master *lessor, struct
idr *leases)
+static struct drm_master *drm_lease_create(struct drm_master *lessor,
+ struct drm_master *lessee)
{
struct drm_device *dev = lessor->dev;
int error;
- struct drm_master *lessee;
- int object;
void *entry;
+ unsigned long index;
DRM_DEBUG_LEASE("lessor %d\n", lessor->lessee_id);
- lessee = drm_master_create(lessor->dev);
- if (!lessee) {
- DRM_DEBUG_LEASE("drm_master_create failed\n");
- return ERR_PTR(-ENOMEM);
- }
-
mutex_lock(&dev->mode_config.idr_mutex);
- idr_for_each_entry(leases, entry, object) {
+ xa_for_each(&lessee->leases, index, entry) {
error = 0;
- if (!idr_find(&dev->mode_config.crtc_idr, object))
+ if (!xa_load(&dev->mode_config.objects, index))
error = -ENOENT;
- else if (!_drm_lease_held_master(lessor, object))
+ else if (!_drm_lease_held_master(lessor, index))
error = -EACCES;
- else if (_drm_has_lease
[PATCH 12/34] drm/agp: Convert bo_list_handles to XArray
Signed-off-by: Matthew Wilcox
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 22 -
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 --
3 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index bcef6ea4bcf9..6a704aaa7dbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -406,8 +406,7 @@ struct amdgpu_fpriv {
struct amdgpu_vmvm;
struct amdgpu_bo_va *prt_va;
struct amdgpu_bo_va *csa_va;
- struct mutexbo_list_lock;
- struct idr bo_list_handles;
+ struct xarray bo_list_handles;
struct amdgpu_ctx_mgr ctx_mgr;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 5c79da8e1150..76439a52a6b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -153,9 +153,7 @@ static void amdgpu_bo_list_destroy(struct amdgpu_fpriv
*fpriv, int id)
{
struct amdgpu_bo_list *list;
- mutex_lock(&fpriv->bo_list_lock);
- list = idr_remove(&fpriv->bo_list_handles, id);
- mutex_unlock(&fpriv->bo_list_lock);
+ list = xa_erase(&fpriv->bo_list_handles, id);
if (list)
kref_put(&list->refcount, amdgpu_bo_list_free);
}
@@ -164,7 +162,7 @@ int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id,
struct amdgpu_bo_list **result)
{
rcu_read_lock();
- *result = idr_find(&fpriv->bo_list_handles, id);
+ *result = xa_load(&fpriv->bo_list_handles, id);
if (*result && kref_get_unless_zero(&(*result)->refcount)) {
rcu_read_unlock();
@@ -278,15 +276,13 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void
*data,
if (r)
goto error_free;
- mutex_lock(&fpriv->bo_list_lock);
- r = idr_alloc(&fpriv->bo_list_handles, list, 1, 0, GFP_KERNEL);
- mutex_unlock(&fpriv->bo_list_lock);
+ r = xa_alloc(&fpriv->bo_list_handles, &handle, list,
+ xa_limit_31b, GFP_KERNEL);
if (r < 0) {
amdgpu_bo_list_put(list);
return r;
}
- handle = r;
break;
case AMDGPU_BO_LIST_OP_DESTROY:
@@ -300,13 +296,11 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void
*data,
if (r)
goto error_free;
- mutex_lock(&fpriv->bo_list_lock);
- old = idr_replace(&fpriv->bo_list_handles, list, handle);
- mutex_unlock(&fpriv->bo_list_lock);
-
- if (IS_ERR(old)) {
+ old = xa_store(&fpriv->bo_list_handles, handle, list,
+ GFP_KERNEL);
+ if (xa_is_err(old)) {
amdgpu_bo_list_put(list);
- r = PTR_ERR(old);
+ r = xa_err(old);
goto error_free;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index bc62bf41b7e9..1a5bf3a4f5d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -986,8 +986,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct
drm_file *file_priv)
goto error_vm;
}
- mutex_init(&fpriv->bo_list_lock);
- idr_init(&fpriv->bo_list_handles);
+ xa_init_flags(&fpriv->bo_list_handles, XA_FLAGS_ALLOC1);
amdgpu_ctx_mgr_init(&fpriv->ctx_mgr);
@@ -1026,7 +1025,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
struct amdgpu_bo_list *list;
struct amdgpu_bo *pd;
unsigned int pasid;
- int handle;
+ unsigned long handle;
if (!fpriv)
return;
@@ -1058,11 +1057,10 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
amdgpu_pasid_free_delayed(pd->tbo.resv, pasid);
amdgpu_bo_unref(&pd);
- idr_for_each_entry(&fpriv->bo_list_handles, list, handle)
+ xa_for_each(&fpriv->bo_list_handles, handle, list)
amdgpu_bo_list_put(list);
- idr_destroy(&fpriv->bo_list_handles);
- mutex_destroy(&fpriv->bo_list_lock);
+ xa_destroy(&fpriv->bo_list_handles);
kfree(fpriv);
file_priv->driver_priv = NULL;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/34] drm: Convert crtc_idr to XArray
On Fri, Feb 22, 2019 at 10:40:14AM +0100, Daniel Vetter wrote: > On Thu, Feb 21, 2019 at 10:41:41AM -0800, Matthew Wilcox wrote: > > - Rename it to 'objects', as requested in todo.rst > > Yay, thanks! > > > - Also convert leases IDR to XArray as the two are occasionally used by > >the same code (see drm_mode_get_lease_ioctl()) > > - Refactor drm_mode_create_lease_ioctl() to create the new drm_master > >early to avoid creating an XArray on the stack and reparenting it > >afterwards. > > All lgtm, also the idr_replace replacement. > > One thing I wonder: For the lesse object xa, we really only store 0/1 in > there, and I don't think that'll change. There was once the idea that we'd > look up objects for a lease directly, bypassing the main object idr. But > that doesn't work due to unregister/hotunplug rules, or at least it would > be pain not worth having. Might be worth it to a lookup structure > optimized for that. Or does XA already autocompress that for us? The > object id are likely fairly compressed, good chance all the ones you need > for a lease will fit into the first 64 id. The XArray doesn't compress the contents of the user pointers. It might be worth augmenting the IDA to have all the functionality you need (there's no ida_test() today, for example). I didn't want to take that on as part of this project, and it's certainly no worse than the IDR. But it's on my radar along with a couple of other places in the kernel that could benefit from the IDA if only it had a couple of extra features (eg the fd table would like to an ida_for_each()). It'd involve changing drm_mode_get_lease_ioctl() and maybe a couple of other places where we use config.objects and leases interchangably, so I wasn't entirely convinced it'd be worth it. > > +++ b/drivers/gpu/drm/drm_auth.c > > @@ -110,7 +110,7 @@ struct drm_master *drm_master_create(struct drm_device > > *dev) > > /* initialize the tree of output resource lessees */ > > master->lessor = NULL; > > master->lessee_id = 0; > > - idr_init(&master->leases); > > + xa_init(&master->leases); > > XA_FALGS_ALLOC1, for safety to make sure we never store 0 (considered > invalid id throughout at least modern drm)? This xarray turns out not to be an allocating XArray, it's just one we store pointers in at a predetermined ID. Marking it as allocating wouldn't be terribly harmful, just slightly inefficient. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/34] drm: Convert drm_minors_idr to XArray
On Fri, Feb 22, 2019 at 10:11:14AM +0100, Daniel Vetter wrote: > On Thu, Feb 21, 2019 at 10:41:21AM -0800, Matthew Wilcox wrote: > > Divide all the indices by 64 to save memory. > > > > Signed-off-by: Matthew Wilcox > > Pretty sure this goes boom. Our char device minor allocation scheme is > > device 0: card0=0, renderD0=64 > device 1: card1=1, renderD1=65 > ... > > I think your scheme aliases all devices with the first one. > > And yes the minor(cardX) + 64 == minor(renderDX) is uapi :-) > > If you want to save space we'd need to move the minor allocation from > drm_minor to drm_device (with a very strange allocation scheme of blocks > of 64 entries, every 128 entries). That would also solve the issue with > the current scheme potentially racing if you load multiple drivers at the > same time (except for drm_global_mutex, but that's more an accident than > intention). Not sure if worth the bother. > > Or maybe coffee hasn't kicked in yet over here and I'm missing something? I'm the one who needed moar coffee. I misread: > > - r = idr_alloc(&drm_minors_idr, > > - NULL, > > - 64 * type, > > - 64 * (type + 1), > > - GFP_NOWAIT); As (64 * type) + 1. So I'll redo this patch. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/34] Convert DRM to XArray
On Fri, Feb 22, 2019 at 10:54:21AM +0100, Daniel Vetter wrote: > > - idr_alloc() returned -ENOSPC, radix_tree_insert() returned -EEXIST. > >xa_insert() and xa_alloc() return -EBUSY. > > I think this will upset a few of our tests. Just written some more for > drm-lease at least, and those check for the ENOSPC. Not sure yet what to > do. If there's real userspace (not just a test suite) which actually relies on the exact errno returned, we can change the places which currently just return the errno to something like: if (err == -EBUSY) return -ENOSPC; return err; There are actually a number of places in the kernel which do the opposite translation today, presumably because having a program print out "No space left on device" was confusing. If it's only the test-suite which cares, presumably the test suite can be changed to treat EBUSY and ENOSPC as being equivalent errno values for a given test. > I did at least read the above, I'll leave all the driver patches for > others. At least for now, any leftovers can be smashed into drm-misc. Thanks! I'll resend after -rc1. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/34] drm: Convert aux_idr to XArray
On Mon, Feb 25, 2019 at 07:57:33PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 21, 2019 at 10:41:23AM -0800, Matthew Wilcox wrote:
> > @@ -49,8 +49,7 @@ struct drm_dp_aux_dev {
> >
> > #define DRM_AUX_MINORS 256
> > #define AUX_MAX_OFFSET (1 << 20)
> > -static DEFINE_IDR(aux_idr);
> > -static DEFINE_MUTEX(aux_idr_mutex);
> > +static DEFINE_XARRAY_ALLOC(aux_xa);
> > static struct class *drm_dp_aux_dev_class;
> > static int drm_dev_major = -1;
[...]
> > static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
> > {
> > + static u32 next;
>
> Is there a particular reason for that being static?
It needs to maintain its value between function calls so that we know
where to start allocating from the next time we call xa_alloc_cyclic().
It can either live here with a short name, or we could put it at file
scope with a descriptive name (probably aux_xa_next). It's up to you
which you think is better; it's your driver.
The IDR embedded the 'next' value in the struct idr, but that was
overhead paid by all users of the IDR rather than the very few that
called idr_alloc_cyclic().
> > + err = xa_alloc_cyclic(&aux_xa, &aux_dev->index, aux_dev,
> > + XA_LIMIT(0, DRM_AUX_MINORS), &next, GFP_KERNEL);
> > + if (err < 0) {
> > kfree(aux_dev);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/34] drm/amdgpu: Convert ctx_handles to XArray
On Mon, Feb 25, 2019 at 04:59:55PM +, Koenig, Christian wrote: > Am 25.02.19 um 17:39 schrieb Matthew Wilcox: > > On Mon, Feb 25, 2019 at 05:07:24PM +0100, Christian König wrote: > >>> -#define AMDGPU_VM_MAX_NUM_CTX4096 > >>> +#define AMDGPU_VM_CTX_LIMIT XA_LIMIT(0, 4095) > >> IIRC we actually use 0 as reserved context value in some places. > > That's OK; the ALLOC1 prevents it from using index 0. > > > > You can change it to be XA_LIMIT(1, 4095) if you think that'll be clearer; > > it'll be slightly less efficient assembly (a 64-bit mov-immediate instead > > of a 32-bit mov-immediate), but it's your driver, so it's up to you. > > A code comment should do as well. > > But thinking more about it please DON'T actually change the > AMDGPU_VM_MAX_NUM_CTX define. > > That one needs to be used to calculate the reserved GPU space to map the > context save area, and I really don't want to deal with a XA_LIMIT in > that calculation :) > > I'm really wondering why that doesn't go up in a boom during compilation Maybe that code isn't merged in Linus' tree yet? I was basing this on 5.0-rc5 and there's no mention of AMDGPU_VM_MAX_NUM_CTX outside these usages. If I'd seen another usage, I wouldn't've changed it. The rebasing I'm going to have to do on -rc1 is going to be epically challenging. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/34] drm/amdgpu: Convert ctx_handles to XArray
On Mon, Feb 25, 2019 at 05:07:24PM +0100, Christian König wrote:
> > -#define AMDGPU_VM_MAX_NUM_CTX 4096
> > +#define AMDGPU_VM_CTX_LIMITXA_LIMIT(0, 4095)
>
> IIRC we actually use 0 as reserved context value in some places.
That's OK; the ALLOC1 prevents it from using index 0.
You can change it to be XA_LIMIT(1, 4095) if you think that'll be clearer;
it'll be slightly less efficient assembly (a 64-bit mov-immediate instead
of a 32-bit mov-immediate), but it's your driver, so it's up to you.
> > @@ -533,22 +533,18 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
> > void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
> > {
> > mutex_init(&mgr->lock);
> > - idr_init(&mgr->ctx_handles);
> > + xa_init_flags(&mgr->ctx_handles, XA_FLAGS_ALLOC1);
> > }
> > void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr)
> > {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/34] drm/agp: Convert bo_list_handles to XArray
On Mon, Feb 25, 2019 at 05:06:18PM +0100, Christian König wrote: > In this one there is a typo in the subject line. Thanks, I'll fix it. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, Apr 02, 2019 at 09:56:08PM +0800, Qiang Yu wrote:
> On Tue, Apr 2, 2019 at 7:26 PM Matthew Wilcox wrote:
> >
> > On Tue, Apr 02, 2019 at 01:55:03PM +0800, Qiang Yu wrote:
> > > Thanks, patch is:
> > > Reviewed-by: Qiang Yu
> >
> > This looks like a fairly naive conversion from the old IDR API to the
> > XArray API. You should be able to remove mgr->lock entirely, relying on
> > the xa_lock for synchronising free and get.
>
> I'm afraid the a little complex free path may involve some might sleep
> functions so use a mutex lock instead of spinlock.
Ah, good call ...
+ mutex_lock(&mgr->lock);
+ ctx = xa_erase(&mgr->handles, id);
+ if (ctx)
+ kref_put(&ctx->refcnt, lima_ctx_do_release);
+ else
+ ret = -EINVAL;
+ mutex_unlock(&mgr->lock);
+static void lima_ctx_do_release(struct kref *ref)
+{
+ struct lima_ctx *ctx = container_of(ref, struct lima_ctx, refcnt);
+ int i;
+
+ for (i = 0; i < lima_pipe_num; i++)
+ lima_sched_context_fini(ctx->dev->pipe + i, ctx->context + i);
+ kfree(ctx);
+}
+void lima_sched_context_fini(struct lima_sched_pipe *pipe,
+struct lima_sched_context *context)
+{
+ drm_sched_entity_fini(&context->base);
+}
and drm_sched_entity_fini() can call kthread_park(), which does sleep.
> > If you think it's worth it,
> > you could even use kfree_rcu() to free the ctx and kref_get_unless_zero()
> > and then your get path would be lock-free.
>
> I can take a look this way, thanks.
I think that's the only way you can get rid of the mutex, given the
sleeping functions called in the free path.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, Apr 02, 2019 at 10:50:06AM +1100, Stephen Rothwell wrote:
> +++ b/drivers/gpu/drm/lima/lima_ctx.c
> @@ -23,7 +23,7 @@ int lima_ctx_create(struct lima_device *dev, struct
> lima_ctx_mgr *mgr, u32 *id)
> goto err_out0;
> }
>
> - err = xa_alloc(&mgr->handles, id, UINT_MAX, ctx, GFP_KERNEL);
> + err = xa_alloc(&mgr->handles, id, ctx, XA_LIMIT(*id, UINT_MAX),
> GFP_KERNEL);
> if (err < 0)
> goto err_out0;
I agree that this is an exact translation of what the code was doing,
but I don't think it's what the author intended the code to do.
They almost certainly meant:
err = xa_alloc(&mgr->handles, id, ctx, xa_limit_32b, GFP_KERNEL);
I'm basing this on:
+struct drm_lima_ctx_create {
+ __u32 id; /* out, context handle */
+ __u32 _pad;/* pad, must be zero */
+};
(and this confusion is exactly why I changed the API ...)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, Apr 02, 2019 at 01:55:03PM +0800, Qiang Yu wrote: > Thanks, patch is: > Reviewed-by: Qiang Yu This looks like a fairly naive conversion from the old IDR API to the XArray API. You should be able to remove mgr->lock entirely, relying on the xa_lock for synchronising free and get. If you think it's worth it, you could even use kfree_rcu() to free the ctx and kref_get_unless_zero() and then your get path would be lock-free. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
On Mon, Jul 22, 2019 at 11:53:54AM -0700, John Hubbard wrote:
> On 7/22/19 2:33 AM, Christoph Hellwig wrote:
> > On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubb...@gmail.com wrote:
> >>for (i = 0; i < vsg->num_pages; ++i) {
> >>if (NULL != (page = vsg->pages[i])) {
> >>if (!PageReserved(page) && (DMA_FROM_DEVICE ==
> >> vsg->direction))
> >> - SetPageDirty(page);
> >> - put_page(page);
> >> + put_user_pages_dirty(&page, 1);
> >> + else
> >> + put_user_page(page);
> >>}
> >
> > Can't just pass a dirty argument to put_user_pages? Also do we really
>
> Yes, and in fact that would help a lot more than the single page case,
> which is really just cosmetic after all.
>
> > need a separate put_user_page for the single page case?
> > put_user_pages_dirty?
>
> Not really. I'm still zeroing in on the ideal API for all these call sites,
> and I agree that the approach below is cleaner.
so enum { CLEAN = 0, DIRTY = 1, LOCK = 2, DIRTY_LOCK = 3 };
?
Re: Report in unix_stream_read_generic()
On Wed, Feb 16, 2022 at 01:17:03PM +0900, Byungchul Park wrote: > [7.013330] === > [7.013331] DEPT: Circular dependency has been detected. > [7.013332] 5.17.0-rc1-00014-gcf3441bb2012 #2 Tainted: GW > [7.01] --- > [7.013334] summary > [7.013334] --- > [7.013335] *** DEADLOCK *** > [7.013335] > [7.013335] context A > [7.013336] [S] (unknown)(&(&ei->socket.wq.wait)->dmap:0) > [7.013337] [W] __mutex_lock_common(&u->iolock:0) > [7.013338] [E] event(&(&ei->socket.wq.wait)->dmap:0) > [7.013340] > [7.013340] context B > [7.013341] [S] __raw_spin_lock(&u->lock:0) > [7.013342] [W] wait(&(&ei->socket.wq.wait)->dmap:0) > [7.013343] [E] spin_unlock(&u->lock:0) This seems unlikely to be real. We're surely not actually waiting while holding a spinlock; existing debug checks would catch it. > [7.013407] --- > [7.013407] context B's detail > [7.013408] --- > [7.013408] context B > [7.013409] [S] __raw_spin_lock(&u->lock:0) > [7.013410] [W] wait(&(&ei->socket.wq.wait)->dmap:0) > [7.013411] [E] spin_unlock(&u->lock:0) > [7.013412] > [7.013412] [S] __raw_spin_lock(&u->lock:0): > [7.013413] [] unix_stream_read_generic+0x6bf/0xb60 > [7.013416] stacktrace: > [7.013416] _raw_spin_lock+0x6e/0x90 > [7.013418] unix_stream_read_generic+0x6bf/0xb60 It would be helpful if you'd run this through scripts/decode_stacktrace.sh so we could see line numbers instead of hex offsets (which arene't much use without the binary kernel). > [7.013420] unix_stream_recvmsg+0x40/0x50 > [7.013422] sock_read_iter+0x85/0xd0 > [7.013424] new_sync_read+0x162/0x180 > [7.013426] vfs_read+0xf3/0x190 > [7.013428] ksys_read+0xa6/0xc0 > [7.013429] do_syscall_64+0x3a/0x90 > [7.013431] entry_SYSCALL_64_after_hwframe+0x44/0xae > [7.013433] > [7.013434] [W] wait(&(&ei->socket.wq.wait)->dmap:0): > [7.013434] [] prepare_to_wait+0x47/0xd0 ... this may be the source of confusion. Just because we prepare to wait doesn't mean we end up actually waiting. For example, look at unix_wait_for_peer(): prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE); sched = !sock_flag(other, SOCK_DEAD) && !(other->sk_shutdown & RCV_SHUTDOWN) && unix_recvq_full(other); unix_state_unlock(other); if (sched) timeo = schedule_timeout(timeo); finish_wait(&u->peer_wait, &wait); We *prepare* to wait, *then* drop the lock, then actually schedule.
Re: Report 1 in ext4 and journal based on v5.17-rc1
On Thu, Feb 17, 2022 at 08:10:03PM +0900, Byungchul Park wrote: > [7.009608] === > [7.009613] DEPT: Circular dependency has been detected. > [7.009614] 5.17.0-rc1-00014-g8a599299c0cb-dirty #30 Tainted: GW > [7.009616] --- > [7.009617] summary > [7.009618] --- > [7.009618] *** DEADLOCK *** > [7.009618] > [7.009619] context A > [7.009619] [S] (unknown)(&(bit_wait_table + i)->dmap:0) Why is the context unknown here? I don't see a way to debug this without knowing where we acquired the bit wait lock. > [7.009621] [W] down_write(&ei->i_data_sem:0) > [7.009623] [E] event(&(bit_wait_table + i)->dmap:0) > [7.009624] > [7.009625] context B > [7.009625] [S] down_read(&ei->i_data_sem:0) > [7.009626] [W] wait(&(bit_wait_table + i)->dmap:0) > [7.009627] [E] up_read(&ei->i_data_sem:0) > [7.009628] > [7.009629] [S]: start of the event context > [7.009629] [W]: the wait blocked > [7.009630] [E]: the event not reachable > [7.009631] --- > [7.009631] context A's detail > [7.009632] --- > [7.009632] context A > [7.009633] [S] (unknown)(&(bit_wait_table + i)->dmap:0) > [7.009634] [W] down_write(&ei->i_data_sem:0) > [7.009635] [E] event(&(bit_wait_table + i)->dmap:0) > [7.009636] > [7.009636] [S] (unknown)(&(bit_wait_table + i)->dmap:0): > [7.009638] (N/A) > [7.009638] > [7.009639] [W] down_write(&ei->i_data_sem:0): > [7.009639] ext4_truncate (fs/ext4/inode.c:4187) > [7.009645] stacktrace: > [7.009646] down_write (kernel/locking/rwsem.c:1514) > [7.009648] ext4_truncate (fs/ext4/inode.c:4187) > [7.009650] ext4_da_write_begin (./include/linux/fs.h:827 > fs/ext4/truncate.h:23 fs/ext4/inode.c:2963) > [7.009652] generic_perform_write (mm/filemap.c:3784) > [7.009654] ext4_buffered_write_iter (fs/ext4/file.c:269) > [7.009657] ext4_file_write_iter (fs/ext4/file.c:677) > [7.009659] new_sync_write (fs/read_write.c:504 (discriminator 1)) > [7.009662] vfs_write (fs/read_write.c:590) > [7.009663] ksys_write (fs/read_write.c:644) > [7.009664] do_syscall_64 (arch/x86/entry/common.c:50 > arch/x86/entry/common.c:80) > [7.009667] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:113) > [7.009669] > [7.009670] [E] event(&(bit_wait_table + i)->dmap:0): > [7.009671] __wake_up_common (kernel/sched/wait.c:108) > [7.009673] stacktrace: > [7.009674] dept_event (kernel/dependency/dept.c:2337) > [7.009677] __wake_up_common (kernel/sched/wait.c:109) > [7.009678] __wake_up_common_lock (./include/linux/spinlock.h:428 > (discriminator 1) kernel/sched/wait.c:141 (discriminator 1)) > [7.009679] __wake_up_bit (kernel/sched/wait_bit.c:127) > [7.009681] ext4_orphan_del (fs/ext4/orphan.c:282) > [7.009683] ext4_truncate (fs/ext4/inode.c:4212) > [7.009685] ext4_da_write_begin (./include/linux/fs.h:827 > fs/ext4/truncate.h:23 fs/ext4/inode.c:2963) > [7.009687] generic_perform_write (mm/filemap.c:3784) > [7.009688] ext4_buffered_write_iter (fs/ext4/file.c:269) > [7.009690] ext4_file_write_iter (fs/ext4/file.c:677) > [7.009692] new_sync_write (fs/read_write.c:504 (discriminator 1)) > [7.009694] vfs_write (fs/read_write.c:590) > [7.009695] ksys_write (fs/read_write.c:644) > [7.009696] do_syscall_64 (arch/x86/entry/common.c:50 > arch/x86/entry/common.c:80) > [7.009698] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:113) > [7.009700] --- > [7.009700] context B's detail > [7.009701] --- > [7.009702] context B > [7.009702] [S] down_read(&ei->i_data_sem:0) > [7.009703] [W] wait(&(bit_wait_table + i)->dmap:0) > [7.009704] [E] up_read(&ei->i_data_sem:0) > [7.009705] > [7.009706] [S] down_read(&ei->i_data_sem:0): > [7.009707] ext4_map_blocks (./arch/x86/include/asm/bitops.h:207 > ./include/asm-generic/bitops/instrumented-non-atomic.h:135 > fs/ext4/ext4.h:1918 fs/ext4/inode.c:562) > [7.009709] stacktrace: > [7.009709] down_read (kernel/locking/rwsem.c:1461) > [7.009711] ext4_map_blocks (./arch/x86/include/asm/bitops.h:207 > ./include/asm-generic/bitops/instrumented-non-atomic.h:135 > fs/ext4/ext4.h:1918 fs/ext4/inode.c:562) > [7.009712] ext4_getblk (fs/ext4/inode.c:851) > [7.009714] ext4_bread (fs/ext4/inode.c:903) > [7.009715] __ext4_read_dirblock (fs/ext4/namei.c:117) > [7.009718] dx_probe (fs/ext4/namei.c:789) > [7.009720] ext4_dx_find_entry (fs/ext4/namei.c:1721) > [7.009722] __ext4_find_entry
Re: [PATCH 00/16] DEPT(Dependency Tracker)
On Thu, Feb 17, 2022 at 12:00:05PM -0500, Steven Rostedt wrote: > On Thu, 17 Feb 2022 10:51:09 -0500 > "Theodore Ts'o" wrote: > > > I know that you're trying to help us, but this tool needs to be far > > better than Lockdep before we should think about merging it. Even if > > it finds 5% more potential deadlocks, if it creates 95% more false > > positive reports --- and the ones it finds are crazy things that > > rarely actually happen in practice, are the costs worth the benefits? > > And who is bearing the costs, and who is receiving the benefits? > > I personally believe that there's potential that this can be helpful and we > will want to merge it. > > But, what I believe Ted is trying to say is, if you do not know if the > report is a bug or not, please do not ask the maintainers to determine it > for you. This is a good opportunity for you to look to see why your tool > reported an issue, and learn that subsystem. Look at if this is really a > bug or not, and investigate why. I agree with Steven here, to the point where I'm willing to invest some time being a beta-tester for this, so if you focus your efforts on filesystem/mm kinds of problems, I can continue looking at them and tell you what's helpful and what's unhelpful in the reports.
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Wed, May 25, 2022 at 03:20:06PM -0700, Andrew Morton wrote: > On Wed, 25 May 2022 23:07:35 +0100 Jessica Clarke wrote: > > > This is i386, so an unsigned long is 32-bit, but i_blocks is a blkcnt_t > > i.e. a u64, which makes the shift without a cast of the LHS fishy. > > Ah, of course, thanks. I remember 32 bits ;) > > --- a/mm/shmem.c~mm-shmemc-suppress-shift-warning > +++ a/mm/shmem.c > @@ -1945,7 +1945,7 @@ alloc_nohuge: > > spin_lock_irq(&info->lock); > info->alloced += folio_nr_pages(folio); > - inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > + inode->i_blocks += (blkcnt_t)BLOCKS_PER_PAGE << folio_order(folio); Bizarre this started showing up now. The recent patch was: - info->alloced += compound_nr(page); - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); + info->alloced += folio_nr_pages(folio); + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); so it could tell that compound_order() was small, but folio_order() might be large? Silencing the warning is a good thing, but folio_order() can (at the moment) be at most 9 on i386, so it isn't actually going to be larger than 4096.
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 11:48:32AM +0300, Dan Carpenter wrote: > On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > > Bizarre this started showing up now. The recent patch was: > > > > - info->alloced += compound_nr(page); > > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > > + info->alloced += folio_nr_pages(folio); > > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > > > so it could tell that compound_order() was small, but folio_order() > > might be large? > > The old code also generates a warning on my test system. Smatch thinks > both compound_order() and folio_order() are 0-255. I guess because of > the "unsigned char compound_order;" in the struct page. It'd be nice if we could annotate that as "contains a value between 1 and BITS_PER_LONG - PAGE_SHIFT". Then be able to optionally enable a checker that ensures that's true on loads/stores. Maybe we need a language that isn't C :-P Ada can do this ... I don't think Rust can.
Re: [PATCH] drm/amdkfd: Add SVM API support capability bits
On Wed, Mar 30, 2022 at 04:24:20PM -0500, Alex Sierra wrote: > From: Philip Yang > > SVMAPISupported property added to HSA_CAPABILITY, the value match > HSA_CAPABILITY defined in Thunk spec: > > SVMAPISupported: it will not be supported on older kernels that don't > have HMM or on systems with GFXv8 or older GPUs without support for > 48-bit virtual addresses. > > CoherentHostAccess property added to HSA_MEMORYPROPERTY, the value match > HSA_MEMORYPROPERTY defined in Thunk spec: > > CoherentHostAccess: whether or not device memory can be coherently > accessed by the host CPU. Could you translate this commit message into English? Reviewing Documentation/process/5.Posting.rst might be helpful.
Re: [Bug 215807] Bad page state in process systemd-udevd
On Tue, Apr 12, 2022 at 02:08:04PM -0700, Andrew Morton wrote: > hm, that's my third `bad page' report in three emails. But I'm not > seeing a pattern - this one might be a DRM thing. I noticed it was an order-9 page and was set to take responsibility for it, but it's clearly not a filesystem page, but a DRM page. Let me help decode this for the benefit of the DRM folks. > > [8.453363] amdgpu: Topology: Add CPU node > > [8.467169] BUG: Bad page state in process systemd-udevd pfn:11b403 > > [8.467180] fbcon: Taking over console > > [8.467188] page:2cc06944 refcount:0 mapcount:0 > > mapping: index:0x3 pfn:0x11b403 > > [8.467195] head:aa25e58e order:9 compound_mapcount:0 > > compound_pincount:0 > > [8.467198] flags: > > 0x17c001(head|node=0|zone=2|lastcpupid=0x1f) > > [8.467205] raw: 0017c000 f2da846d0001 f2da846d00c8 > > > > [8.467208] raw: > > > > [8.467210] head: 0017c001 dead0122 > > > > [8.467212] head: > > > > [8.467214] page dumped because: corrupted mapping in tail page Tail pages are all supposed to have page->mapping == TAIL_MAPPING (0x400 + POISON_POINTER_DELTA). This one has page->mapping == NULL. I say "all", but tail pages 1 and 2 aren't checked because those get other useful things stored in them instead. This is tail page number 3, so it's the first one checked. So DRM has probably been overly enthusiastic about cleaning up something. Hope that's helpful!
Re: [PATCH v3 0/4] drm: Use full allocated minor range for DRM
On Tue, Sep 06, 2022 at 04:01:13PM +0200, Michał Winiarski wrote: > 64 DRM device nodes is not enough for everyone. > Upgrade it to ~512K (which definitely is more than enough). > > To allow testing userspace support for >64 devices, add additional DRM > modparam (skip_legacy_minors) which causes DRM to skip allocating minors > in 0-192 range. > Additionally - one minor tweak around minor DRM IDR locking and IDR lockdep > annotations. The IDR is deprecated; rather than making all these changes around the IDR, could you convert it to use the XArray instead? I did it once before, but those patches bounced off the submissions process.
Re: [PATCH v4 1/3] drm: Use XArray instead of IDR for minors
On Tue, Sep 06, 2022 at 10:16:27PM +0200, Michał Winiarski wrote:
> IDR is deprecated, and since XArray manages its own state with internal
> locking, it simplifies the locking on DRM side.
> Additionally, don't use the IRQ-safe variant, since operating on drm
> minor is not done in IRQ context.
>
> Signed-off-by: Michał Winiarski
> Suggested-by: Matthew Wilcox
I have a few questions, but I like where you're going.
> @@ -98,21 +98,18 @@ static struct drm_minor **drm_minor_get_slot(struct
> drm_device *dev,
> static void drm_minor_alloc_release(struct drm_device *dev, void *data)
> {
> struct drm_minor *minor = data;
> - unsigned long flags;
>
> WARN_ON(dev != minor->dev);
>
> put_device(minor->kdev);
>
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_remove(&drm_minors_idr, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + xa_release(&drm_minors_xa, minor->index);
Has it definitely been unused at this point? I would think that
xa_erase() (an unconditional store) would be the correct function to
call.
> @@ -122,20 +119,12 @@ static int drm_minor_alloc(struct drm_device *dev,
> unsigned int type)
> minor->type = type;
> minor->dev = dev;
>
> - idr_preload(GFP_KERNEL);
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - r = idr_alloc(&drm_minors_idr,
> - NULL,
> - 64 * type,
> - 64 * (type + 1),
> - GFP_NOWAIT);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> - idr_preload_end();
> -
> + r = xa_alloc(&drm_minors_xa, &id, NULL,
> + XA_LIMIT(64 * type, 64 * (type + 1) - 1), GFP_KERNEL);
> if (r < 0)
> return r;
>
> - minor->index = r;
> + minor->index = id;
Wouldn't it be better to call:
r = xa_alloc(&drm_minors_xa, &minor->index, NULL,
XA_LIMIT(64 * type, 64 * (type + 1) - 1), GFP_KERNEL);
I might also prefer a little syntactic sugar like:
#define DRM_MINOR_LIMIT(type) XA_LIMIT(64 * (type), 64 * (type) + 63)
but that's definitely a matter of taste.
> @@ -172,9 +161,12 @@ static int drm_minor_register(struct drm_device *dev,
> unsigned int type)
> goto err_debugfs;
>
> /* replace NULL with @minor so lookups will succeed from now on */
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_replace(&drm_minors_idr, minor, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + entry = xa_store(&drm_minors_xa, minor->index, &minor, GFP_KERNEL);
> + if (xa_is_err(entry)) {
> + ret = xa_err(entry);
> + goto err_debugfs;
> + }
> + WARN_ON(entry);
Might be better as an xa_cmpxchg()?
> @@ -187,16 +179,13 @@ static int drm_minor_register(struct drm_device *dev,
> unsigned int type)
> static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> {
> struct drm_minor *minor;
> - unsigned long flags;
>
> minor = *drm_minor_get_slot(dev, type);
> if (!minor || !device_is_registered(minor->kdev))
> return;
>
> /* replace @minor with NULL so lookups will fail from now on */
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_replace(&drm_minors_idr, NULL, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + xa_erase(&drm_minors_xa, minor->index);
This isn't an exact replacement, but I'm not sure whether that makes a
difference. xa_erase() allows allocation of this ID again while
idr_replace() means that lookups return NULL, but the ID remains in
use. The equivalent of idr_replace() is:
xa_store(&drm_minors_xa, minor->index, NULL, GFP_KERNEL);
> @@ -215,13 +204,10 @@ static void drm_minor_unregister(struct drm_device
> *dev, unsigned int type)
> struct drm_minor *drm_minor_acquire(unsigned int minor_id)
> {
> struct drm_minor *minor;
> - unsigned long flags;
>
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - minor = idr_find(&drm_minors_idr, minor_id);
> + minor = xa_load(&drm_minors_xa, minor_id);
> if (minor)
> drm_dev_get(minor->dev);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
This is also not an exact equivalent as the dev_drm_get() is now outside
the lock. Does that matter in this case? I don't know the code well
enough to say. If you want it to be equivalent, then:
xa_lock(&drm_minors_xa);
minor = xa_load(&d
Re: [PATCH v8 07/15] mm/gup: migrate device coherent pages when pinning instead of failing
On Mon, Jul 11, 2022 at 03:35:42PM +0200, David Hildenbrand wrote:
> > + /*
> > +* Device coherent pages are managed by a driver and should not
> > +* be pinned indefinitely as it prevents the driver moving the
> > +* page. So when trying to pin with FOLL_LONGTERM instead try
> > +* to migrate the page out of device memory.
> > +*/
> > + if (folio_is_device_coherent(folio)) {
> > + WARN_ON_ONCE(PageCompound(&folio->page));
>
> Maybe that belongs into migrate_device_page()?
... and should be simply WARN_ON_ONCE(folio_test_large(folio)).
No need to go converting it back into a page in order to test things
that can't possibly be true.
Re: [PATCH] mm: Fix a null ptr deref with CONFIG_DEBUG_VM enabled in wp_page_reuse
On Wed, Jul 27, 2022 at 03:14:07PM -0400, Zack Rusin wrote: > From: Zack Rusin > > Write page faults on last references might not have a valid page anymore. > wp_page_reuse has always dealt with that scenario by making > sure the page isn't null (or the reference was shared) before doing > anything with it. Recently added checks in VM_BUG_ON (enabled by the > CONFIG_DEBUG_VM option) use PageAnon helpers which assume the passed > page is never null, before making sure there is a valid page to work > with. > > Move the VM_BUG_ON, which unconditionally uses the page, after the > code that checks that we have a valid one. Message-ID:
