<div dir="ltr"><div dir="ltr">On Thu, Jun 6, 2019 at 7:57 PM NeilBrown <<a href="mailto:neilb@suse.com">neilb@suse.com</a>> wrote:<br>><br>> On Thu, Jun 06 2019, Shaun Tancheff wrote:<br>><br>> > Ensure all uses of cl_page->cp_state are smp-safe.<br>> ><br>> > LBUG Output:<br>> > In __cl_page_state_set()<br>> > ..<br>> >   old = page->cp_state;<br>> >   PASSERT(env, page, allowed_transitions[old][state]);<br>> > ..<br>> > Asserted with the following:<br>> >   page@ffff80be1fcd6600[3 ffff80be1fca5cc0 0 1 ffff809e60e2 8cc0]<br>><br>> What do all these fields mean?  Or where is this printed?<br>> The closest I can find is in cl_page_header_print() which uses the<br>> format string:<br>><br>>                    "page@%p[%d %p %d %d %p]\n",<br>><br>> But that only has 5 fields in the [], while the output you provided has<br>> 6.<br>><br><br>Sorry for the confusion, here is my (long) explanation with source snippets:<br><br>The LBUG in full<br><br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) page@ffff80be1fcd6600[3 ffff80be1fca5cc0 0 1 ffff809e60e28cc0]<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) vvp-page@ffff80be1fcd6650(0:0) vm@ffff7e02fa0e60c0 5000000000001029 4:0 ffff80be1fcd6600 1049548 lru<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) lov-page@ffff80be1fcd6690, comp index: 0, gen: 0<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) osc-page@ffff80be1fcd66c8 1049548: 1< 0x845fed 2 0 - - > 2< 4298948608 0 4096 0x0 0x420 |           (null) ffff809e473708e8 ffff80be199c1e40 > 3< 0 0 0 > 4< 5 13 64 0 + | - - - - > 5< - - - - | 0 - | 0 - -><br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) end page@ffff80be1fcd6600<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) allowed_transitions[old][state]<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) ASSERTION( 0 ) failed:<br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) LBUG<br><br>The source snippet around L342<br>...<br>   339 <br>   340          ENTRY;<br>   341          old = page->cp_state;<br>   342          PASSERT(env, page, allowed_transitions[old][state]);<br>   343          CL_PAGE_HEADER(D_TRACE, env, page, "%d -> %d\n", old, state);<br>   344          PASSERT(env, page, page->cp_state == old);<br>   345          PASSERT(env, page, equi(state == CPS_OWNED, page->cp_owner != NULL));<br>   346 <br>   347          cs_pagestate_dec(page->cp_obj, page->cp_state);<br>   348          cs_pagestate_inc(page->cp_obj, state);<br><br>The PASSERT macro ...<br>...<br># define PASSERT(env, page, expr)                                       \<br>  do {                                                                    \<br>          if (unlikely(!(expr))) {                                      \<br>                  CL_PAGE_DEBUG(D_ERROR, (env), (page), #expr "\n");    \<br>                  LASSERT(0);                                           \<br>          }                                                             \<br>  } while (0)<br>#else /* !LIBCFS_DEBUG */<br>...<br><br>/**<br> * Helper macro, dumping detailed information about \a page into a log.<br> */<br>#define CL_PAGE_DEBUG(mask, env, page, format, ...)                     \<br>do {                                                                    \<br>        if (cfs_cdebug_show(mask, DEBUG_SUBSYSTEM)) {                   \<br>                LIBCFS_DEBUG_MSG_DATA_DECL(msgdata, mask, NULL);        \<br>                cl_page_print(env, &msgdata, lu_cdebug_printer, page);  \<br>                CDEBUG(mask, format , ## __VA_ARGS__);                  \<br>        }                                                               \<br>} while (0)<br><br>Almost there<br>...<br>  1062  /**<br>  1063   * Prints human readable representation of \a pg to the \a f.<br>  1064   */<br>  1065  void cl_page_print(const struct lu_env *env, void *cookie,<br>  1066                     lu_printer_t printer, const struct cl_page *pg)<br>  1067  {<br>  1068          const struct cl_page_slice *slice;<br>  1069          int result = 0;<br>  1070 <br>  1071          cl_page_header_print(env, cookie, printer, pg);<br>  1072          list_for_each_entry(slice, &pg->cp_layers, cpl_linkage) {<br>  1073                  if (slice->cpl_ops->cpo_print != NULL)<br>  1074                          result = (*slice->cpl_ops->cpo_print)(env, slice,<br>  1075                                                               cookie, printer);<br><br>Finally the 'good stuff'<br>...<br>  1051  void cl_page_header_print(const struct lu_env *env, void *cookie,<br>  1052                            lu_printer_t printer, const struct cl_page *pg)<br>  1053  {<br>  1054          (*printer)(env, cookie,<br>  1055                     "page@%p[%d %p %d %d %p]\n",<br>  1056                     pg, atomic_read(&pg->cp_ref), pg->cp_obj,<br>  1057                     pg->cp_state, pg->cp_type,<br>  1058                     pg->cp_owner);<br>  1059  }<br>...<br><br>Given the above along with the first line from the LBUG ....<div><br>LustreError: 86407:0:(cl_page.c:342:cl_page_state_set0()) page@ffff80be1fcd6600[3 ffff80be1fca5cc0 0 1 ffff809e60e28cc0]<br><br>Implies the following:</div><div><br>pg == ffff80be1fcd6600<br>pg->cp_ref == 3<br>pg->cp_obj == ffff80be1fca5cc0<br>pg->cp_state == 0 (state == CPS_CACHED)<br>pg->cp_type == 1<br>pg->cp_owner == ffff809e60e28cc0<br><br>Also known caller was cl_page_assume+0xdc/0x3e0 [obdclass] (from the backtrace)<br>So we know the on-stack / register value of state == CPS_OWNED</div><div><br></div><div>The backtrace:<br>...<br> dump_backtrace+0x0/0x248<br> show_stack+0x24/0x30<br> dump_stack+0xbc/0xf4<br> libcfs_call_trace+0xec/0x120 [libcfs]<br> lbug_with_loc+0x4c/0xb8 [libcfs]<br> cl_page_state_set0+0x2b4/0x6e0 [obdclass]<br> cl_page_assume+0xdc/0x3e0 [obdclass]<br> ll_io_read_page+0x144c/0x1de0 [lustre]<br> ll_write_begin+0x3d4/0xee8 [lustre]<br><br></div><div>and the contents of cl_page_assume</div><div><br>   596  void cl_page_assume(const struct lu_env *env,<br>   597                      struct cl_io *io, struct cl_page *pg)<br>   598  {<br>   599          const struct cl_page_slice *slice;<br>   600 <br>   601          PINVRNT(env, pg, cl_object_same(pg->cp_obj, io->ci_obj));<br>   602 <br>   603          ENTRY;<br>   604          io = cl_io_top(io);<br>   605 <br>   606          list_for_each_entry(slice, &pg->cp_layers, cpl_linkage) {<br>   607                  if (slice->cpl_ops->cpo_assume != NULL)<br>   608                          (*slice->cpl_ops->cpo_assume)(env, slice, io);<br>   609          }<br>   610 <br>   611          PASSERT(env, pg, pg->cp_owner == NULL);<br>   612          pg->cp_owner = cl_io_top(io);<br>   613          cl_page_owner_set(pg);<br>   614          cl_page_state_set(env, pg, CPS_OWNED);<br>   615          EXIT;<br>   616  }<br><br></div><div>While I believe my conclusion to be correct I am certainly open to being dead wrong.</div><div><br></div></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 6, 2019 at 7:57 PM NeilBrown <<a href="mailto:neilb@suse.com">neilb@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jun 06 2019, Shaun Tancheff wrote:<br>
<br>
> Ensure all uses of cl_page->cp_state are smp-safe.<br>
><br>
> LBUG Output:<br>
> In __cl_page_state_set()<br>
> ..<br>
>   old = page->cp_state;<br>
>   PASSERT(env, page, allowed_transitions[old][state]);<br>
> ..<br>
> Asserted with the following:<br>
>   page@ffff80be1fcd6600[3 ffff80be1fca5cc0 0 1 ffff809e60e2 8cc0]<br>
<br>
What do all these fields mean?  Or where is this printed?<br>
The closest I can find is in cl_page_header_print() which uses the<br>
format string:<br>
<br>
                   "page@%p[%d %p %d %d %p]\n",<br>
<br>
But that only has 5 fields in the [], while the output you provided has<br>
6.<br>
<br>
><br>
> However cp_state 0 (CPS_CACHED) to 1 (CPS_OWNED) is a valid transition<br>
> leading to the conclusion that cp_state became 0 during the<br>
> assertion.<br>
<br>
Where is the evidence that this was the transition that was happening?<br>
If it was in some part of the LBUG output that wasn't quoted - then<br>
please fix that by quoting the entire LBUG output.<br>
<br>
We need to understand which change happened at this time to cause the<br>
race.<br>
Then we need to explain why the added barriers actually close the race<br>
window.<br>
<br>
If these barrier do actually fix a race, then there is something *very*<br>
wrong.<br>
The comments say that changes to cp_state are protected by the page lock<br>
on the corresponding VM page.<br>
Locking and unlocking a VM page entails sufficient barriers that changes<br>
made while one thread holds the lock will be visible to another<br>
thread once it also gets the lock.<br>
<br></blockquote><div><br></div><div>True, but a page lock is just a bit on the page flags. And we absolutely wait on I/O while the </div><div>page is 'locked' so a ping-pong (our task *can* sleep) is entirely reasonable, as far as</div><div>I understand anyway. Since there is multiple state-changes during a single page lock/unlock</div><div>there is a race, however rare and unlikely, and it's arch specific based on what I understand</div><div>of the memory barrier rules.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
So the only possible explanation for a race as you suspect, is if the<br>
vmpage *isn't* locked when something changes cp_state, and that would be<br>
bad.<br>
<br>
Thanks,<br>
NeilBrown<br>
<br>
<br>
><br>
> Signed-off-by: Shaun Tancheff <<a href="mailto:stancheff@cray.com" target="_blank">stancheff@cray.com</a>><br>
> ---<br>
>  fs/lustre/include/cl_object.h   | 11 +++++++++++<br>
>  fs/lustre/llite/rw26.c          |  2 +-<br>
>  fs/lustre/llite/vvp_page.c      |  6 +++---<br>
>  fs/lustre/obdclass/cl_page.c    | 34 +++++++++++++++++++--------------<br>
>  fs/lustre/obdecho/echo_client.c |  2 +-<br>
>  fs/lustre/osc/osc_cache.c       | 18 +++++++++--------<br>
>  6 files changed, 46 insertions(+), 27 deletions(-)<br>
><br>
> diff --git a/fs/lustre/include/cl_object.h b/fs/lustre/include/cl_object.h<br>
> index 691c2f5da53a..d6e1f6f05f50 100644<br>
> --- a/fs/lustre/include/cl_object.h<br>
> +++ b/fs/lustre/include/cl_object.h<br>
> @@ -752,6 +752,17 @@ struct cl_page {<br>
>       struct cl_sync_io               *cp_sync_io;<br>
>  };<br>
>  <br>
> +static inline enum cl_page_state cl_page_state_get(const struct cl_page *pg)<br>
> +{<br>
> +     /*<br>
> +      * Paired with smp_store_release in cl_page_state_set_trust<br>
> +      * and ensures that we see the most recent value of cp_state<br>
> +      * even when the last modification was not performed on the<br>
> +      * current processor<br>
> +      */<br>
> +     return smp_load_acquire(&pg->cp_state);<br>
> +}<br>
> +<br>
>  /**<br>
>   * Per-layer part of cl_page.<br>
>   *<br>
> diff --git a/fs/lustre/llite/rw26.c b/fs/lustre/llite/rw26.c<br>
> index e4ce3b6f5772..364dec208ccd 100644<br>
> --- a/fs/lustre/llite/rw26.c<br>
> +++ b/fs/lustre/llite/rw26.c<br>
> @@ -200,7 +200,7 @@ static ssize_t ll_direct_IO_seg(const struct lu_env *env, struct cl_io *io,<br>
>  <br>
>               rc = cl_page_own(env, io, clp);<br>
>               if (rc) {<br>
> -                     LASSERT(clp->cp_state == CPS_FREEING);<br>
> +                     LASSERT(cl_page_state_get(clp) == CPS_FREEING);<br>
>                       cl_page_put(env, clp);<br>
>                       break;<br>
>               }<br>
> diff --git a/fs/lustre/llite/vvp_page.c b/fs/lustre/llite/vvp_page.c<br>
> index 590e5f5e43c9..38b8c488d765 100644<br>
> --- a/fs/lustre/llite/vvp_page.c<br>
> +++ b/fs/lustre/llite/vvp_page.c<br>
> @@ -323,18 +323,18 @@ static int vvp_page_make_ready(const struct lu_env *env,<br>
>  <br>
>       lock_page(vmpage);<br>
>       if (clear_page_dirty_for_io(vmpage)) {<br>
> -             LASSERT(pg->cp_state == CPS_CACHED);<br>
> +             LASSERT(cl_page_state_get(pg) == CPS_CACHED);<br>
>               /* This actually clears the dirty bit in the radix tree. */<br>
>               set_page_writeback(vmpage);<br>
>               CL_PAGE_HEADER(D_PAGE, env, pg, "readied\n");<br>
> -     } else if (pg->cp_state == CPS_PAGEOUT) {<br>
> +     } else if (cl_page_state_get(pg) == CPS_PAGEOUT) {<br>
>               /* is it possible for osc_flush_async_page() to already<br>
>                * make it ready?<br>
>                */<br>
>               result = -EALREADY;<br>
>       } else {<br>
>               CL_PAGE_DEBUG(D_ERROR, env, pg, "Unexpecting page state %d.\n",<br>
> -                           pg->cp_state);<br>
> +                           cl_page_state_get(pg));<br>
>               LBUG();<br>
>       }<br>
>       unlock_page(vmpage);<br>
> diff --git a/fs/lustre/obdclass/cl_page.c b/fs/lustre/obdclass/cl_page.c<br>
> index 349f19e014e0..da4429b82932 100644<br>
> --- a/fs/lustre/obdclass/cl_page.c<br>
> +++ b/fs/lustre/obdclass/cl_page.c<br>
> @@ -97,7 +97,7 @@ static void cl_page_free(const struct lu_env *env, struct cl_page *page)<br>
>  <br>
>       PASSERT(env, page, list_empty(&page->cp_batch));<br>
>       PASSERT(env, page, !page->cp_owner);<br>
> -     PASSERT(env, page, page->cp_state == CPS_FREEING);<br>
> +     PASSERT(env, page, cl_page_state_get(page) == CPS_FREEING);<br>
>  <br>
>       while ((slice = list_first_entry_or_null(&page->cp_layers,<br>
>                                                struct cl_page_slice,<br>
> @@ -119,8 +119,14 @@ static void cl_page_free(const struct lu_env *env, struct cl_page *page)<br>
>  static inline void cl_page_state_set_trust(struct cl_page *page,<br>
>                                          enum cl_page_state state)<br>
>  {<br>
> -     /* bypass const. */<br>
> -     *(enum cl_page_state *)&page->cp_state = state;<br>
> +     /*<br>
> +      * Paired with smp_load_acquire in cl_page_state_get<br>
> +      * and ensures that we see the most recent value of cp_state<br>
> +      * is available even when the next access is not performed on the<br>
> +      * current processor.<br>
> +      * Note we also cast away const as the only modifier of cp_state.<br>
> +      */<br>
> +     smp_store_release((enum cl_page_state *)&page->cp_state, state);<br>
>  }<br>
>  <br>
>  struct cl_page *cl_page_alloc(const struct lu_env *env,<br>
> @@ -270,10 +276,10 @@ static void __cl_page_state_set(const struct lu_env *env,<br>
>               }<br>
>       };<br>
>  <br>
> -     old = page->cp_state;<br>
> +     old = cl_page_state_get(page);<br>
>       PASSERT(env, page, allowed_transitions[old][state]);<br>
>       CL_PAGE_HEADER(D_TRACE, env, page, "%d -> %d\n", old, state);<br>
> -     PASSERT(env, page, page->cp_state == old);<br>
> +     PASSERT(env, page, cl_page_state_get(page) == old);<br>
>       PASSERT(env, page, equi(state == CPS_OWNED, page->cp_owner));<br>
>       cl_page_state_set_trust(page, state);<br>
>  }<br>
> @@ -313,7 +319,7 @@ void cl_page_put(const struct lu_env *env, struct cl_page *page)<br>
>                      refcount_read(&page->cp_ref));<br>
>  <br>
>       if (refcount_dec_and_test(&page->cp_ref)) {<br>
> -             LASSERT(page->cp_state == CPS_FREEING);<br>
> +             LASSERT(cl_page_state_get(page) == CPS_FREEING);<br>
>  <br>
>               LASSERT(refcount_read(&page->cp_ref) == 0);<br>
>               PASSERT(env, page, !page->cp_owner);<br>
> @@ -378,7 +384,7 @@ void __cl_page_disown(const struct lu_env *env,<br>
>       const struct cl_page_slice *slice;<br>
>       enum cl_page_state state;<br>
>  <br>
> -     state = pg->cp_state;<br>
> +     state = cl_page_state_get(pg);<br>
>       cl_page_owner_clear(pg);<br>
>  <br>
>       if (state == CPS_OWNED)<br>
> @@ -402,7 +408,7 @@ int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)<br>
>       struct cl_io *top = cl_io_top((struct cl_io *)io);<br>
>  <br>
>       LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));<br>
> -     return pg->cp_state == CPS_OWNED && pg->cp_owner == top;<br>
> +     return cl_page_state_get(pg) == CPS_OWNED && pg->cp_owner == top;<br>
>  }<br>
>  EXPORT_SYMBOL(cl_page_is_owned);<br>
>  <br>
> @@ -434,7 +440,7 @@ static int __cl_page_own(const struct lu_env *env, struct cl_io *io,<br>
>  <br>
>       io = cl_io_top(io);<br>
>  <br>
> -     if (pg->cp_state == CPS_FREEING) {<br>
> +     if (cl_page_state_get(pg) == CPS_FREEING) {<br>
>               result = -ENOENT;<br>
>               goto out;<br>
>       }<br>
> @@ -453,7 +459,7 @@ static int __cl_page_own(const struct lu_env *env, struct cl_io *io,<br>
>               PASSERT(env, pg, !pg->cp_owner);<br>
>               pg->cp_owner = cl_io_top(io);<br>
>               cl_page_owner_set(pg);<br>
> -             if (pg->cp_state != CPS_FREEING) {<br>
> +             if (cl_page_state_get(pg) != CPS_FREEING) {<br>
>                       cl_page_state_set(env, pg, CPS_OWNED);<br>
>               } else {<br>
>                       __cl_page_disown(env, io, pg);<br>
> @@ -593,7 +599,7 @@ static void __cl_page_delete(const struct lu_env *env, struct cl_page *pg)<br>
>  {<br>
>       const struct cl_page_slice *slice;<br>
>  <br>
> -     PASSERT(env, pg, pg->cp_state != CPS_FREEING);<br>
> +     PASSERT(env, pg, cl_page_state_get(pg) != CPS_FREEING);<br>
>  <br>
>       /*<br>
>        * Sever all ways to obtain new pointers to @pg.<br>
> @@ -756,7 +762,7 @@ void cl_page_completion(const struct lu_env *env,<br>
>       const struct cl_page_slice *slice;<br>
>  <br>
>       PASSERT(env, pg, crt < CRT_NR);<br>
> -     PASSERT(env, pg, pg->cp_state == cl_req_type_state(crt));<br>
> +     PASSERT(env, pg, cl_page_state_get(pg) == cl_req_type_state(crt));<br>
>  <br>
>       CL_PAGE_HEADER(D_TRACE, env, pg, "%d %d\n", crt, ioret);<br>
>  <br>
> @@ -805,7 +811,7 @@ int cl_page_make_ready(const struct lu_env *env, struct cl_page *pg,<br>
>       }<br>
>  <br>
>       if (result >= 0) {<br>
> -             PASSERT(env, pg, pg->cp_state == CPS_CACHED);<br>
> +             PASSERT(env, pg, cl_page_state_get(pg) == CPS_CACHED);<br>
>               cl_page_io_start(env, pg, crt);<br>
>               result = 0;<br>
>       }<br>
> @@ -870,7 +876,7 @@ void cl_page_header_print(const struct lu_env *env, void *cookie,<br>
>       (*printer)(env, cookie,<br>
>                  "page@%p[%d %p %d %d %p]\n",<br>
>                  pg, refcount_read(&pg->cp_ref), pg->cp_obj,<br>
> -                pg->cp_state, pg->cp_type,<br>
> +                cl_page_state_get(pg), pg->cp_type,<br>
>                  pg->cp_owner);<br>
>  }<br>
>  EXPORT_SYMBOL(cl_page_header_print);<br>
> diff --git a/fs/lustre/obdecho/echo_client.c b/fs/lustre/obdecho/echo_client.c<br>
> index 317123fd27cb..d879f109e641 100644<br>
> --- a/fs/lustre/obdecho/echo_client.c<br>
> +++ b/fs/lustre/obdecho/echo_client.c<br>
> @@ -1046,7 +1046,7 @@ static int cl_echo_object_brw(struct echo_object *eco, int rw, u64 offset,<br>
>  <br>
>               rc = cl_page_own(env, io, clp);<br>
>               if (rc) {<br>
> -                     LASSERT(clp->cp_state == CPS_FREEING);<br>
> +                     LASSERT(cl_page_state_get(clp) == CPS_FREEING);<br>
>                       cl_page_put(env, clp);<br>
>                       break;<br>
>               }<br>
> diff --git a/fs/lustre/osc/osc_cache.c b/fs/lustre/osc/osc_cache.c<br>
> index f8fddbfe6a7e..75984b98b229 100644<br>
> --- a/fs/lustre/osc/osc_cache.c<br>
> +++ b/fs/lustre/osc/osc_cache.c<br>
> @@ -1045,7 +1045,7 @@ static int osc_extent_truncate(struct osc_extent *ext, pgoff_t trunc_index,<br>
>                       cl_page_discard(env, io, page);<br>
>                       cl_page_disown(env, io, page);<br>
>               } else {<br>
> -                     LASSERT(page->cp_state == CPS_FREEING);<br>
> +                     LASSERT(cl_page_state_get(page) == CPS_FREEING);<br>
>                       LASSERT(0);<br>
>               }<br>
>  <br>
> @@ -1356,10 +1356,12 @@ static int osc_completion(const struct lu_env *env, struct osc_async_page *oap,<br>
>       int srvlock;<br>
>  <br>
>       cmd &= ~OBD_BRW_NOQUOTA;<br>
> -     LASSERTF(equi(page->cp_state == CPS_PAGEIN, cmd == OBD_BRW_READ),<br>
> -              "cp_state:%u, cmd:%d\n", page->cp_state, cmd);<br>
> -     LASSERTF(equi(page->cp_state == CPS_PAGEOUT, cmd == OBD_BRW_WRITE),<br>
> -              "cp_state:%u, cmd:%d\n", page->cp_state, cmd);<br>
> +     LASSERTF(equi(cl_page_state_get(page) == CPS_PAGEIN,<br>
> +                   cmd == OBD_BRW_READ),<br>
> +              "cp_state:%u, cmd:%d\n", cl_page_state_get(page), cmd);<br>
> +     LASSERTF(equi(cl_page_state_get(page) == CPS_PAGEOUT,<br>
> +                   cmd == OBD_BRW_WRITE),<br>
> +              "cp_state:%u, cmd:%d\n", cl_page_state_get(page), cmd);<br>
>       LASSERT(opg->ops_transfer_pinned);<br>
>  <br>
>       crt = cmd == OBD_BRW_READ ? CRT_READ : CRT_WRITE;<br>
> @@ -3061,7 +3063,7 @@ bool osc_page_gang_lookup(const struct lu_env *env, struct cl_io *io,<br>
>  <br>
>                       page = ops->ops_cl.cpl_page;<br>
>                       LASSERT(page->cp_type == CPT_CACHEABLE);<br>
> -                     if (page->cp_state == CPS_FREEING)<br>
> +                     if (cl_page_state_get(page) == CPS_FREEING)<br>
>                               continue;<br>
>  <br>
>                       cl_page_get(page);<br>
> @@ -3142,7 +3144,7 @@ static bool check_and_discard_cb(const struct lu_env *env, struct cl_io *io,<br>
>                       cl_page_discard(env, io, page);<br>
>                       cl_page_disown(env, io, page);<br>
>               } else {<br>
> -                     LASSERT(page->cp_state == CPS_FREEING);<br>
> +                     LASSERT(cl_page_state_get(page) == CPS_FREEING);<br>
>               }<br>
>       }<br>
>  <br>
> @@ -3169,7 +3171,7 @@ static bool discard_cb(const struct lu_env *env, struct cl_io *io,<br>
>               cl_page_discard(env, io, page);<br>
>               cl_page_disown(env, io, page);<br>
>       } else {<br>
> -             LASSERT(page->cp_state == CPS_FREEING);<br>
> +             LASSERT(cl_page_state_get(page) == CPS_FREEING);<br>
>       }<br>
>  <br>
>       return true;<br>
> -- <br>
> 2.17.1<br>
</blockquote></div></div>