| Message ID | 20201125162532.1299794-5-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: don't set page->mapping | expand |
On Wed, Nov 25, 2020 at 5:25 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Random observation while trying to review Christian's patch series to > stop looking at struct page for dma-buf imports. > > This was originally added in > > commit 58aa6622d32af7d2c08d45085f44c54554a16ed7 > Author: Thomas Hellstrom <thellstrom@vmware.com> > Date: Fri Jan 3 11:47:23 2014 +0100 > > drm/ttm: Correctly set page mapping and -index members > > Needed for some vm operations; most notably unmap_mapping_range() with > even_cows = 0. > > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> > Reviewed-by: Brian Paul <brianp@vmware.com> > > but we do not have a single caller of unmap_mapping_range with > even_cows == 0. And all the gem drivers don't do this, so another > small thing we could standardize between drm and ttm drivers. > > Plus I don't really see a need for unamp_mapping_range where we don't > want to indiscriminately shoot down all ptes. > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Cc: Brian Paul <brianp@vmware.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Christian Koenig <christian.koenig@amd.com> > Cc: Huang Rui <ray.huang@amd.com> Apologies again, this shouldn't have been included. But at least I have an idea now why this patch somehow was included in the git send-email. Lovely interface :-/ -Daniel > --- > drivers/gpu/drm/ttm/ttm_tt.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index da9eeffe0c6d..5b2eb6d58bb7 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -284,17 +284,6 @@ int ttm_tt_swapout(struct ttm_bo_device *bdev, struct ttm_tt *ttm) > return ret; > } > > -static void ttm_tt_add_mapping(struct ttm_bo_device *bdev, struct ttm_tt *ttm) > -{ > - pgoff_t i; > - > - if (ttm->page_flags & TTM_PAGE_FLAG_SG) > - return; > - > - for (i = 0; i < ttm->num_pages; ++i) > - ttm->pages[i]->mapping = bdev->dev_mapping; > -} > - > int ttm_tt_populate(struct ttm_bo_device *bdev, > struct ttm_tt *ttm, struct ttm_operation_ctx *ctx) > { > @@ -313,7 +302,6 @@ int ttm_tt_populate(struct ttm_bo_device *bdev, > if (ret) > return ret; > > - ttm_tt_add_mapping(bdev, ttm); > ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED; > if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { > ret = ttm_tt_swapin(ttm); > -- > 2.29.2 >
On Wed, Nov 25, 2020 at 05:28:32PM +0100, Daniel Vetter wrote: > On Wed, Nov 25, 2020 at 5:25 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > Random observation while trying to review Christian's patch series to > > stop looking at struct page for dma-buf imports. > > > > This was originally added in > > > > commit 58aa6622d32af7d2c08d45085f44c54554a16ed7 > > Author: Thomas Hellstrom <thellstrom@vmware.com> > > Date: Fri Jan 3 11:47:23 2014 +0100 > > > > drm/ttm: Correctly set page mapping and -index members > > > > Needed for some vm operations; most notably unmap_mapping_range() with > > even_cows = 0. > > > > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> > > Reviewed-by: Brian Paul <brianp@vmware.com> > > > > but we do not have a single caller of unmap_mapping_range with > > even_cows == 0. And all the gem drivers don't do this, so another > > small thing we could standardize between drm and ttm drivers. > > > > Plus I don't really see a need for unamp_mapping_range where we don't > > want to indiscriminately shoot down all ptes. > > > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > Cc: Brian Paul <brianp@vmware.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Christian Koenig <christian.koenig@amd.com> > > Cc: Huang Rui <ray.huang@amd.com> > > Apologies again, this shouldn't have been included. But at least I > have an idea now why this patch somehow was included in the git > send-email. Lovely interface :-/ I wrote a bit of a script around this because git send-email just too hard to use The key workflow change I made was to have it prepare all the emails to send and open them in an editor for review - exactly as they would be sent to the lists. It uses a empty 'cover-letter' commit and automatically transforms it into exactly the right stuff. Keeps track of everything you send in git, and there is a little tool to auto-run git range-diff to help build change logs.. https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_send_patches.py I've been occasionaly wondering if I should suggest Konstantin add a sending side to b4, maybe using some of those ideas.. (careful if you run it, it does autosend without prompting) Jason
On Wed, Nov 25, 2020 at 02:06:06PM -0400, Jason Gunthorpe wrote: > It uses a empty 'cover-letter' commit and automatically transforms it > into exactly the right stuff. Keeps track of everything you send in > git, and there is a little tool to auto-run git range-diff to help > build change logs.. > > https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_send_patches.py > > I've been occasionaly wondering if I should suggest Konstantin add a > sending side to b4, maybe using some of those ideas.. > > (careful if you run it, it does autosend without prompting) The looks pretty fancy. Here is my trivial patchbomb.sh script ----------------------- snip ----------------------- #!/bin/sh COVERLETTER=$1 PATCHES=$2 git send-email --annotate --to-cover --cc-cover $1 $2 ----------------------- snip ----------------------- still needs the git basecommit..endcommit notation, but it fires up the series for review.
Hi, On Wed, 25 Nov 2020 at 18:06, Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Wed, Nov 25, 2020 at 05:28:32PM +0100, Daniel Vetter wrote: > > Apologies again, this shouldn't have been included. But at least I > > have an idea now why this patch somehow was included in the git > > send-email. Lovely interface :-/ > > I wrote a bit of a script around this because git send-email just too > hard to use > > The key workflow change I made was to have it prepare all the emails > to send and open them in an editor for review - exactly as they would > be sent to the lists. > > It uses a empty 'cover-letter' commit and automatically transforms it > into exactly the right stuff. Keeps track of everything you send in > git, and there is a little tool to auto-run git range-diff to help > build change logs.. This sounds a fair bit like patman, which does something similar and also lets you annotate commit messages with changelogs. But of course, suggesting different methods of carving patches into stone tablets to someone who's written their own, is even more of a windmill tilt than rDMA. ;) Cheers, Daniel
On Wed, Nov 25, 2020 at 06:11:29PM +0000, Christoph Hellwig wrote: > On Wed, Nov 25, 2020 at 02:06:06PM -0400, Jason Gunthorpe wrote: > > It uses a empty 'cover-letter' commit and automatically transforms it > > into exactly the right stuff. Keeps track of everything you send in > > git, and there is a little tool to auto-run git range-diff to help > > build change logs.. > > > > https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_send_patches.py > > > > I've been occasionaly wondering if I should suggest Konstantin add a > > sending side to b4, maybe using some of those ideas.. > > > > (careful if you run it, it does autosend without prompting) > > The looks pretty fancy. Here is my trivial patchbomb.sh script > > #!/bin/sh > > COVERLETTER=$1 > PATCHES=$2 > > git send-email --annotate --to-cover --cc-cover $1 $2 > > still needs the git basecommit..endcommit notation, but it fires > up the series for review. annotate is OK, I used that for a long time.. My main gripe was it didn't setup the to/cc until after the annotate editor closes. Jason
On Wed, Nov 25, 2020 at 07:57:20PM -0400, Jason Gunthorpe wrote: > annotate is OK, I used that for a long time.. > > My main gripe was it didn't setup the to/cc until after the annotate > editor closes. I put the To/Cc into the cover letter text file.
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index da9eeffe0c6d..5b2eb6d58bb7 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -284,17 +284,6 @@ int ttm_tt_swapout(struct ttm_bo_device *bdev, struct ttm_tt *ttm) return ret; } -static void ttm_tt_add_mapping(struct ttm_bo_device *bdev, struct ttm_tt *ttm) -{ - pgoff_t i; - - if (ttm->page_flags & TTM_PAGE_FLAG_SG) - return; - - for (i = 0; i < ttm->num_pages; ++i) - ttm->pages[i]->mapping = bdev->dev_mapping; -} - int ttm_tt_populate(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_operation_ctx *ctx) { @@ -313,7 +302,6 @@ int ttm_tt_populate(struct ttm_bo_device *bdev, if (ret) return ret; - ttm_tt_add_mapping(bdev, ttm); ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED; if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) { ret = ttm_tt_swapin(ttm);
Random observation while trying to review Christian's patch series to stop looking at struct page for dma-buf imports. This was originally added in commit 58aa6622d32af7d2c08d45085f44c54554a16ed7 Author: Thomas Hellstrom <thellstrom@vmware.com> Date: Fri Jan 3 11:47:23 2014 +0100 drm/ttm: Correctly set page mapping and -index members Needed for some vm operations; most notably unmap_mapping_range() with even_cows = 0. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Reviewed-by: Brian Paul <brianp@vmware.com> but we do not have a single caller of unmap_mapping_range with even_cows == 0. And all the gem drivers don't do this, so another small thing we could standardize between drm and ttm drivers. Plus I don't really see a need for unamp_mapping_range where we don't want to indiscriminately shoot down all ptes. Cc: Thomas Hellstrom <thellstrom@vmware.com> Cc: Brian Paul <brianp@vmware.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Christian Koenig <christian.koenig@amd.com> Cc: Huang Rui <ray.huang@amd.com> --- drivers/gpu/drm/ttm/ttm_tt.c | 12 ------------ 1 file changed, 12 deletions(-)