<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Understood, thanks. The fact that it's used for page locking elsewhere in Linux is plenty for me, was just curious.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Thanks,</p>
<p style="margin-top:0;margin-bottom:0">- Patrick</p>
<br>
<br>
<div style="color: rgb(0, 0, 0);">
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> NeilBrown <neilb@suse.com><br>
<b>Sent:</b> Sunday, December 9, 2018 7:26 PM<br>
<b>To:</b> Patrick Farrell; James Simmons; Oleg Drokin; Andreas Dilger<br>
<b>Cc:</b> Lustre Development List<br>
<b>Subject:</b> Re: [lustre-devel] [PATCH 3/4] lustre: use bit-locking in echo_client.</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Mon, Dec 10 2018, Patrick Farrell wrote:<br>
<br>
> Neil,<br>
><br>
> So the semantics and behavior for a single bit bit lock and a mutex are the same? All the scheduler stuff, optimistic spin, etc? It may not matter here (probably not) but it makes me curious...<br>
<br>
No, not exactly the same - but close enough in this case.<br>
<br>
We don't get the optimistic spinning, and if there are multiple waiters<br>
they will all be woken when the lock is released, instead of just one.<br>
<br>
What we gain is a locking mechanism used in echo_client that is nearly<br>
identical to the locking mechansim (lock_page()) that is used for the<br>
normal Linux client.<br>
<br>
So yes, there are minor differences, I don't think they are important.<br>
I should have clarified that in the commit message.<br>
<br>
Thanks,<br>
NeilBrown<br>
<br>
<br>
><br>
> - Patrick<br>
><br>
> ________________________________<br>
> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of NeilBrown <neilb@suse.com><br>
> Sent: Sunday, December 9, 2018 6:46:16 PM<br>
> To: James Simmons; Oleg Drokin; Andreas Dilger<br>
> Cc: Lustre Development List<br>
> Subject: [lustre-devel] [PATCH 3/4] lustre: use bit-locking in echo_client.<br>
><br>
> The ep_lock used by echo client causes lockdep to complain.<br>
> Multiple locks of the same class are taken concurrently which<br>
> appear to lockdep to be prone to deadlocking, and can fill up<br>
> lockdep's fixed size stack for locks.<br>
><br>
> Ass ep_lock is taken on multiple pages always in ascending page order,<br>
> deadlocks don't happen, so this is a false-positive.<br>
><br>
> The function of the ep_lock is the same as thats for page_lock(),<br>
> which is implemented as a bit-lock using wait_on_bit(). lockdep<br>
> cannot see these locks, and doesn't really need to.<br>
><br>
> So convert ep_lock to a simple bit-lock using wait_on_bit for<br>
> waiting. This provides similar functionality, matches how page_lock()<br>
> works, and avoids lockdep problems.<br>
><br>
> Signed-off-by: NeilBrown <neilb@suse.com><br>
> ---<br>
> .../staging/lustre/lustre/obdecho/echo_client.c | 29 +++++++++++++-------<br>
> 1 file changed, 19 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c<br>
> index 1ddb4a6dd8f3..887df7ce6b5c 100644<br>
> --- a/drivers/staging/lustre/lustre/obdecho/echo_client.c<br>
> +++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c<br>
> @@ -78,7 +78,7 @@ struct echo_object_conf {<br>
><br>
> struct echo_page {<br>
> struct cl_page_slice ep_cl;<br>
> - struct mutex ep_lock;<br>
> + unsigned long ep_lock;<br>
> };<br>
><br>
> struct echo_lock {<br>
> @@ -217,10 +217,13 @@ static int echo_page_own(const struct lu_env *env,<br>
> {<br>
> struct echo_page *ep = cl2echo_page(slice);<br>
><br>
> - if (!nonblock)<br>
> - mutex_lock(&ep->ep_lock);<br>
> - else if (!mutex_trylock(&ep->ep_lock))<br>
> - return -EAGAIN;<br>
> + if (nonblock) {<br>
> + if (test_and_set_bit(0, &ep->ep_lock))<br>
> + return -EAGAIN;<br>
> + } else {<br>
> + while (test_and_set_bit(0, &ep->ep_lock))<br>
> + wait_on_bit(&ep->ep_lock, 0, TASK_UNINTERRUPTIBLE);<br>
> + }<br>
> return 0;<br>
> }<br>
><br>
> @@ -230,8 +233,8 @@ static void echo_page_disown(const struct lu_env *env,<br>
> {<br>
> struct echo_page *ep = cl2echo_page(slice);<br>
><br>
> - LASSERT(mutex_is_locked(&ep->ep_lock));<br>
> - mutex_unlock(&ep->ep_lock);<br>
> + LASSERT(test_bit(0, &ep->ep_lock));<br>
> + clear_and_wake_up_bit(0, &ep->ep_lock);<br>
> }<br>
><br>
> static void echo_page_discard(const struct lu_env *env,<br>
> @@ -244,7 +247,7 @@ static void echo_page_discard(const struct lu_env *env,<br>
> static int echo_page_is_vmlocked(const struct lu_env *env,<br>
> const struct cl_page_slice *slice)<br>
> {<br>
> - if (mutex_is_locked(&cl2echo_page(slice)->ep_lock))<br>
> + if (test_bit(0, &cl2echo_page(slice)->ep_lock))<br>
> return -EBUSY;<br>
> return -ENODATA;<br>
> }<br>
> @@ -279,7 +282,7 @@ static int echo_page_print(const struct lu_env *env,<br>
> struct echo_page *ep = cl2echo_page(slice);<br>
><br>
> (*printer)(env, cookie, LUSTRE_ECHO_CLIENT_NAME "-page@%p %d vm@%p\n",<br>
> - ep, mutex_is_locked(&ep->ep_lock),<br>
> + ep, test_bit(0, &ep->ep_lock),<br>
> slice->cpl_page->cp_vmpage);<br>
> return 0;<br>
> }<br>
> @@ -339,7 +342,13 @@ static int echo_page_init(const struct lu_env *env, struct cl_object *obj,<br>
> struct echo_object *eco = cl2echo_obj(obj);<br>
><br>
> get_page(page->cp_vmpage);<br>
> - mutex_init(&ep->ep_lock);<br>
> + /*<br>
> + * ep_lock is similar to the lock_page() lock, and<br>
> + * cannot usefully be monitored by lockdep.<br>
> + * So just a bit in an "unsigned long" and use the<br>
> + * wait_on_bit() interface to wait for the bit to be clera.<br>
> + */<br>
> + ep->ep_lock = 0;<br>
> cl_page_slice_add(page, &ep->ep_cl, obj, index, &echo_page_ops);<br>
> atomic_inc(&eco->eo_npages);<br>
> return 0;<br>
><br>
><br>
> _______________________________________________<br>
> lustre-devel mailing list<br>
> lustre-devel@lists.lustre.org<br>
> <a href="http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org" id="LPlnk668479" class="OWAAutoLink" previewremoved="true">
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org</a>
<div id="LPBorder_GT_15444149028870.6582061015285363" style="margin-bottom: 20px; overflow: auto; width: 100%; text-indent: 0px;">
<table id="LPContainer_15444149028840.5490482717682608" role="presentation" cellspacing="0" style="width: 90%; background-color: rgb(255, 255, 255); position: relative; overflow: auto; padding-top: 20px; padding-bottom: 20px; margin-top: 20px; border-top: 1px dotted rgb(200, 200, 200); border-bottom: 1px dotted rgb(200, 200, 200);">
<tbody>
<tr valign="top" style="border-spacing: 0px;">
<td id="TextCell_15444149028850.006744044874555932" colspan="2" style="vertical-align: top; position: relative; padding: 0px; display: table-cell;">
<div id="LPRemovePreviewContainer_15444149028860.4480656383161399"></div>
<div id="LPTitle_15444149028860.9357260002399461" style="top: 0px; color: rgb(0, 114, 198); font-weight: 400; font-size: 21px; font-family: wf_segoe-ui_light, "Segoe UI Light", "Segoe WP Light", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; line-height: 21px;">
<a id="LPUrlAnchor_15444149028860.6191653030655315" href="http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org" target="_blank" style="text-decoration: none;">lustre-devel Info Page</a></div>
<div id="LPMetadata_15444149028860.4927041601254665" style="margin: 10px 0px 16px; color: rgb(102, 102, 102); font-weight: 400; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 14px;">
lists.lustre.org</div>
<div id="LPDescription_15444149028870.1822831398061011" style="display: block; color: rgb(102, 102, 102); font-weight: 400; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 20px; max-height: 100px; overflow: hidden;">
To see the collection of prior postings to the list, visit the lustre-devel Archives.. Using lustre-devel: To post a message to all the list members, send email to lustre-devel@lists.lustre.org. You can subscribe to the list, or change your existing subscription,
in the sections below.</div>
</td>
</tr>
</tbody>
</table>
</div>
<br>
<br>
</div>
</span></font></div>
</div>
</div>
</body>
</html>