* [PATCH] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
@ 2019-03-21 8:33 Dan Carpenter
2019-03-21 8:58 ` Mika Kuoppala
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-03-21 8:33 UTC (permalink / raw)
To: Jani Nikula, Chris Wilson
Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
The mock_context() function returns NULL on error, it doesn't return
error pointers.
Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
.../gpu/drm/i915/selftests/i915_gem_context.c | 4 +-
drivers/staging/erofs/unzip_vle.c | 70 ++++++++++---------
2 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 4399ef9ebf15..a172dbd9cb9e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -1620,8 +1620,8 @@ static int mock_context_barrier(void *arg)
mutex_lock(&i915->drm.struct_mutex);
ctx = mock_context(i915, "mock");
- if (IS_ERR(ctx)) {
- err = PTR_ERR(ctx);
+ if (!ctx) {
+ err = -ENOMEM;
goto unlock;
}
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index c7b3b21123c1..a711bf61f60c 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -844,11 +844,9 @@ static void z_erofs_vle_unzip_kickoff(void *ptr, int bios)
static inline void z_erofs_vle_read_endio(struct bio *bio)
{
const blk_status_t err = bio->bi_status;
+ struct erofs_sb_info *sbi = NULL;
unsigned int i;
struct bio_vec *bvec;
-#ifdef EROFS_FS_HAS_MANAGED_CACHE
- struct address_space *mc = NULL;
-#endif
struct bvec_iter_all iter_all;
bio_for_each_segment_all(bvec, bio, i, iter_all) {
@@ -858,20 +856,12 @@ static inline void z_erofs_vle_read_endio(struct bio *bio)
DBG_BUGON(PageUptodate(page));
DBG_BUGON(!page->mapping);
-#ifdef EROFS_FS_HAS_MANAGED_CACHE
- if (unlikely(!mc && !z_erofs_is_stagingpage(page))) {
- struct inode *const inode = page->mapping->host;
- struct super_block *const sb = inode->i_sb;
-
- mc = MNGD_MAPPING(EROFS_SB(sb));
- }
+ if (unlikely(!sbi && !z_erofs_is_stagingpage(page)))
+ sbi = EROFS_SB(page->mapping->host->i_sb);
- /*
- * If mc has not gotten, it equals NULL,
- * however, page->mapping never be NULL if working properly.
- */
- cachemngd = (page->mapping = mc);
-#endif
+ /* sbi should already be gotten if the page is managed */
+ if (sbi)
+ cachemngd = erofs_page_is_managed(sbi, page);
if (unlikely(err))
SetPageError(page);
@@ -972,6 +962,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
overlapped = false;
compressed_pages = grp->compressed_pages;
+ err = 0;
for (i = 0; i < clusterpages; ++i) {
unsigned int pagenr;
@@ -981,26 +972,37 @@ static int z_erofs_vle_unzip(struct super_block *sb,
DBG_BUGON(!page);
DBG_BUGON(!page->mapping);
- if (z_erofs_is_stagingpage(page))
- continue;
-#ifdef EROFS_FS_HAS_MANAGED_CACHE
- if (page->mapping = MNGD_MAPPING(sbi)) {
- DBG_BUGON(!PageUptodate(page));
- continue;
- }
-#endif
+ if (!z_erofs_is_stagingpage(page)) {
+ if (erofs_page_is_managed(sbi, page)) {
+ if (unlikely(!PageUptodate(page)))
+ err = -EIO;
+ continue;
+ }
+
+ /*
+ * only if non-head page can be selected
+ * for inplace decompression
+ */
+ pagenr = z_erofs_onlinepage_index(page);
- /* only non-head page could be reused as a compressed page */
- pagenr = z_erofs_onlinepage_index(page);
+ DBG_BUGON(pagenr >= nr_pages);
+ DBG_BUGON(pages[pagenr]);
+ ++sparsemem_pages;
+ pages[pagenr] = page;
- DBG_BUGON(pagenr >= nr_pages);
- DBG_BUGON(pages[pagenr]);
- ++sparsemem_pages;
- pages[pagenr] = page;
+ overlapped = true;
+ }
- overlapped = true;
+ /* PG_error needs checking for inplaced and staging pages */
+ if (unlikely(PageError(page))) {
+ DBG_BUGON(PageUptodate(page));
+ err = -EIO;
+ }
}
+ if (unlikely(err))
+ goto out;
+
llen = (nr_pages << PAGE_SHIFT) - work->pageofs;
if (z_erofs_vle_workgrp_fmt(grp) = Z_EROFS_VLE_WORKGRP_FMT_PLAIN) {
@@ -1044,10 +1046,9 @@ static int z_erofs_vle_unzip(struct super_block *sb,
for (i = 0; i < clusterpages; ++i) {
page = compressed_pages[i];
-#ifdef EROFS_FS_HAS_MANAGED_CACHE
- if (page->mapping = MNGD_MAPPING(sbi))
+ if (erofs_page_is_managed(sbi, page))
continue;
-#endif
+
/* recycle all individual staging pages */
(void)z_erofs_gather_if_stagingpage(page_pool, page);
@@ -1198,6 +1199,7 @@ pickup_page_for_submission(struct z_erofs_vle_workgroup *grp,
if (page->mapping = mc) {
WRITE_ONCE(grp->compressed_pages[nr], page);
+ ClearPageError(page);
if (!PagePrivate(page)) {
/*
* impossible to be !PagePrivate(page) for
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
2019-03-21 8:33 [PATCH] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier() Dan Carpenter
@ 2019-03-21 8:58 ` Mika Kuoppala
2019-03-21 9:22 ` Dan Carpenter
2019-03-21 9:24 ` [PATCH v2] " Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Mika Kuoppala @ 2019-03-21 8:58 UTC (permalink / raw)
To: Dan Carpenter, Jani Nikula, Chris Wilson
Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
Dan Carpenter <dan.carpenter@oracle.com> writes:
> The mock_context() function returns NULL on error, it doesn't return
> error pointers.
>
> Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> .../gpu/drm/i915/selftests/i915_gem_context.c | 4 +-
> drivers/staging/erofs/unzip_vle.c | 70 ++++++++++---------
> 2 files changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 4399ef9ebf15..a172dbd9cb9e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -1620,8 +1620,8 @@ static int mock_context_barrier(void *arg)
> mutex_lock(&i915->drm.struct_mutex);
>
> ctx = mock_context(i915, "mock");
> - if (IS_ERR(ctx)) {
> - err = PTR_ERR(ctx);
> + if (!ctx) {
> + err = -ENOMEM;
> goto unlock;
> }
Yup.
The rest of the diff is unrelated tho, please resend.
-Mika
>
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> index c7b3b21123c1..a711bf61f60c 100644
> --- a/drivers/staging/erofs/unzip_vle.c
> +++ b/drivers/staging/erofs/unzip_vle.c
> @@ -844,11 +844,9 @@ static void z_erofs_vle_unzip_kickoff(void *ptr, int bios)
> static inline void z_erofs_vle_read_endio(struct bio *bio)
> {
> const blk_status_t err = bio->bi_status;
> + struct erofs_sb_info *sbi = NULL;
> unsigned int i;
> struct bio_vec *bvec;
> -#ifdef EROFS_FS_HAS_MANAGED_CACHE
> - struct address_space *mc = NULL;
> -#endif
> struct bvec_iter_all iter_all;
>
> bio_for_each_segment_all(bvec, bio, i, iter_all) {
> @@ -858,20 +856,12 @@ static inline void z_erofs_vle_read_endio(struct bio *bio)
> DBG_BUGON(PageUptodate(page));
> DBG_BUGON(!page->mapping);
>
> -#ifdef EROFS_FS_HAS_MANAGED_CACHE
> - if (unlikely(!mc && !z_erofs_is_stagingpage(page))) {
> - struct inode *const inode = page->mapping->host;
> - struct super_block *const sb = inode->i_sb;
> -
> - mc = MNGD_MAPPING(EROFS_SB(sb));
> - }
> + if (unlikely(!sbi && !z_erofs_is_stagingpage(page)))
> + sbi = EROFS_SB(page->mapping->host->i_sb);
>
> - /*
> - * If mc has not gotten, it equals NULL,
> - * however, page->mapping never be NULL if working properly.
> - */
> - cachemngd = (page->mapping = mc);
> -#endif
> + /* sbi should already be gotten if the page is managed */
> + if (sbi)
> + cachemngd = erofs_page_is_managed(sbi, page);
>
> if (unlikely(err))
> SetPageError(page);
> @@ -972,6 +962,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> overlapped = false;
> compressed_pages = grp->compressed_pages;
>
> + err = 0;
> for (i = 0; i < clusterpages; ++i) {
> unsigned int pagenr;
>
> @@ -981,26 +972,37 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> DBG_BUGON(!page);
> DBG_BUGON(!page->mapping);
>
> - if (z_erofs_is_stagingpage(page))
> - continue;
> -#ifdef EROFS_FS_HAS_MANAGED_CACHE
> - if (page->mapping = MNGD_MAPPING(sbi)) {
> - DBG_BUGON(!PageUptodate(page));
> - continue;
> - }
> -#endif
> + if (!z_erofs_is_stagingpage(page)) {
> + if (erofs_page_is_managed(sbi, page)) {
> + if (unlikely(!PageUptodate(page)))
> + err = -EIO;
> + continue;
> + }
> +
> + /*
> + * only if non-head page can be selected
> + * for inplace decompression
> + */
> + pagenr = z_erofs_onlinepage_index(page);
>
> - /* only non-head page could be reused as a compressed page */
> - pagenr = z_erofs_onlinepage_index(page);
> + DBG_BUGON(pagenr >= nr_pages);
> + DBG_BUGON(pages[pagenr]);
> + ++sparsemem_pages;
> + pages[pagenr] = page;
>
> - DBG_BUGON(pagenr >= nr_pages);
> - DBG_BUGON(pages[pagenr]);
> - ++sparsemem_pages;
> - pages[pagenr] = page;
> + overlapped = true;
> + }
>
> - overlapped = true;
> + /* PG_error needs checking for inplaced and staging pages */
> + if (unlikely(PageError(page))) {
> + DBG_BUGON(PageUptodate(page));
> + err = -EIO;
> + }
> }
>
> + if (unlikely(err))
> + goto out;
> +
> llen = (nr_pages << PAGE_SHIFT) - work->pageofs;
>
> if (z_erofs_vle_workgrp_fmt(grp) = Z_EROFS_VLE_WORKGRP_FMT_PLAIN) {
> @@ -1044,10 +1046,9 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> for (i = 0; i < clusterpages; ++i) {
> page = compressed_pages[i];
>
> -#ifdef EROFS_FS_HAS_MANAGED_CACHE
> - if (page->mapping = MNGD_MAPPING(sbi))
> + if (erofs_page_is_managed(sbi, page))
> continue;
> -#endif
> +
> /* recycle all individual staging pages */
> (void)z_erofs_gather_if_stagingpage(page_pool, page);
>
> @@ -1198,6 +1199,7 @@ pickup_page_for_submission(struct z_erofs_vle_workgroup *grp,
> if (page->mapping = mc) {
> WRITE_ONCE(grp->compressed_pages[nr], page);
>
> + ClearPageError(page);
> if (!PagePrivate(page)) {
> /*
> * impossible to be !PagePrivate(page) for
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
2019-03-21 8:58 ` Mika Kuoppala
@ 2019-03-21 9:22 ` Dan Carpenter
2019-03-21 9:24 ` [PATCH v2] " Dan Carpenter
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-03-21 9:22 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
On Thu, Mar 21, 2019 at 10:58:40AM +0200, Mika Kuoppala wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > The mock_context() function returns NULL on error, it doesn't return
> > error pointers.
> >
> > Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > .../gpu/drm/i915/selftests/i915_gem_context.c | 4 +-
> > drivers/staging/erofs/unzip_vle.c | 70 ++++++++++---------
> > 2 files changed, 38 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > index 4399ef9ebf15..a172dbd9cb9e 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > @@ -1620,8 +1620,8 @@ static int mock_context_barrier(void *arg)
> > mutex_lock(&i915->drm.struct_mutex);
> >
> > ctx = mock_context(i915, "mock");
> > - if (IS_ERR(ctx)) {
> > - err = PTR_ERR(ctx);
> > + if (!ctx) {
> > + err = -ENOMEM;
> > goto unlock;
> > }
>
> Yup.
>
> The rest of the diff is unrelated tho, please resend.
>
Oh, crap. Sorry! It was below the bottom of my page in my email client
so I didn't see it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
2019-03-21 8:58 ` Mika Kuoppala
2019-03-21 9:22 ` Dan Carpenter
@ 2019-03-21 9:24 ` Dan Carpenter
2019-03-21 10:01 ` Mika Kuoppala
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2019-03-21 9:24 UTC (permalink / raw)
To: Jani Nikula, Chris Wilson
Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
The mock_context() function returns NULL on error, it doesn't return
error pointers.
Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
V2: I accidentally sent a bunch of unrelated stuff...
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 4399ef9ebf15..a172dbd9cb9e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -1620,8 +1620,8 @@ static int mock_context_barrier(void *arg)
mutex_lock(&i915->drm.struct_mutex);
ctx = mock_context(i915, "mock");
- if (IS_ERR(ctx)) {
- err = PTR_ERR(ctx);
+ if (!ctx) {
+ err = -ENOMEM;
goto unlock;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
2019-03-21 9:24 ` [PATCH v2] " Dan Carpenter
@ 2019-03-21 10:01 ` Mika Kuoppala
2019-03-21 13:31 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2019-03-21 10:01 UTC (permalink / raw)
To: Dan Carpenter, Jani Nikula, Chris Wilson
Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
Dan Carpenter <dan.carpenter@oracle.com> writes:
> The mock_context() function returns NULL on error, it doesn't return
> error pointers.
>
> Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> V2: I accidentally sent a bunch of unrelated stuff...
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 4399ef9ebf15..a172dbd9cb9e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -1620,8 +1620,8 @@ static int mock_context_barrier(void *arg)
> mutex_lock(&i915->drm.struct_mutex);
>
> ctx = mock_context(i915, "mock");
> - if (IS_ERR(ctx)) {
> - err = PTR_ERR(ctx);
> + if (!ctx) {
> + err = -ENOMEM;
> goto unlock;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier()
2019-03-21 10:01 ` Mika Kuoppala
@ 2019-03-21 13:31 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2019-03-21 13:31 UTC (permalink / raw)
To: Dan Carpenter, Jani Nikula, Mika Kuoppala
Cc: David Airlie, intel-gfx, kernel-janitors, Matthew Auld
Quoting Mika Kuoppala (2019-03-21 10:01:13)
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > The mock_context() function returns NULL on error, it doesn't return
> > error pointers.
> >
> > Fixes: 85fddf0b0027 ("drm/i915: Introduce a context barrier callback")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
And pushed, thanks for the fix!
-Chris
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-21 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-21 8:33 [PATCH] drm/i915/selftests: fix NULL vs IS_ERR() check in mock_context_barrier() Dan Carpenter
2019-03-21 8:58 ` Mika Kuoppala
2019-03-21 9:22 ` Dan Carpenter
2019-03-21 9:24 ` [PATCH v2] " Dan Carpenter
2019-03-21 10:01 ` Mika Kuoppala
2019-03-21 13:31 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).