Message ID | 87k39n6mtn.fsf@oracle.com |
---|---|
State | Committed |
Headers |
Return-Path: <x14307373@homiemail-mx20.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 79EEC3600C2 for <siddhesh@wilcox.dreamhost.com>; Thu, 15 May 2014 09:10:58 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14307373) id 2761641DA6B34; Thu, 15 May 2014 09:10:58 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id CC59B41D3032C for <glibc@patchwork.siddhesh.in>; Thu, 15 May 2014 09:10:57 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=mwBdMcvOTj21DF3t rOcpmL7H50KT9Humw8CQo746BZ4U/F1BPIy9MYWt7808aCmGdWuLeTViYzk3VbKa yQ5Rg6y/RgA4qkv55DCLoKdAope1OsMrAYTwPr+WkbuPq963dZxYus7Ypidr6ALL TV15KVt+39zbX6nJrDy5WYUAPW0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; s=default; bh=mfPMkwiI2h/JGsjDQrLkUS cO3t8=; b=YmzeqfQXmqSBBUxKcLbaY3THTI/F84K4IOA5YMJ68HCbadaJRSci// UI8lC3c01cUCqf/xaRXMGCiPxi6Lu89NNkYPIJC3g+lodxXxKH9Ff82IwCslp/yM BfC6oMVt7CALLC+i2HVMl+zOTqBQckI6Qvkv8xK9JCo11ZBJiJ1v4= Received: (qmail 29587 invoked by alias); 15 May 2014 16:10:55 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 29578 invoked by uid 89); 15 May 2014 16:10:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS, UNPARSEABLE_RELAY autolearn=ham version=3.3.2 X-HELO: userp1040.oracle.com From: jose.marchesi@oracle.com (Jose E. Marchesi) To: libc-alpha@sourceware.org Cc: davem@davemloft.net Subject: [PATCH][SPARC] missing membar in niagara2_memcpy Date: Thu, 15 May 2014 18:14:44 +0200 Message-ID: <87k39n6mtn.fsf@oracle.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Jose E. Marchesi
May 15, 2014, 4:14 p.m. UTC
Hi. The following patch prevents stores made before calling memcpy to overlap with the block stores in niagara2_memcpy. Tested on an Athena machine (sparc64x) running sparc64-*-linux-gnu. No new warnings triggered by the patch. No regressions detected. PS: virtually the same code lives in the kernel in linux/arch/sparc/lib/NG2memcpy.S. Probably that file shall be updated too... 2014-05-15 Jose E. Marchesi <jose.marchesi@oracle.com> * sysdeps/sparc/sparc64/multiarch/memcpy-niagara2.S: Add missing membar to avoid block loads/stores to overlap previous stores.
Comments
From: jose.marchesi@oracle.com (Jose E. Marchesi) Date: Thu, 15 May 2014 18:14:44 +0200 > The following patch prevents stores made before calling memcpy to > overlap with the block stores in niagara2_memcpy. > > Tested on an Athena machine (sparc64x) running sparc64-*-linux-gnu. > No new warnings triggered by the patch. > No regressions detected. Does it really fix anything? Are these crashes or data corruptions or thread data visibilty issues that you've seen in practice and are cured by this change? The member is extremely expensive and I avoided it intentionally.
> Tested on an Athena machine (sparc64x) running sparc64-*-linux-gnu. > No new warnings triggered by the patch. > No regressions detected. Does it really fix anything? Certainly. Are these crashes or data corruptions or thread data visibilty issues that you've seen in practice and are cured by this change? Several system tests of a certain (very data intensive) program crashed as soon as it started to use multilib/niagara2_memcpy, due to memory corruption. They run just fine after this patch is applied. We also observed that jar would occasionally raise java.util.zip.ZipException. This was due to the fact the last two bytes of a buffer were not apparently being copied by memcpy. Again, this patch fixes that problem. The member is extremely expensive and I avoided it intentionally. Yes I suspected that, given how carefully that code is written. We are aware that membars are painful on the T2 (any membar, because all flavors are mapped to #Sync) but the risk of overlaps is not only theoretical in this case, unfortunately :(
From: jose.marchesi@oracle.com (Jose E. Marchesi) Date: Thu, 15 May 2014 19:31:43 +0200 > Several system tests of a certain (very data intensive) program crashed > as soon as it started to use multilib/niagara2_memcpy, due to memory > corruption. They run just fine after this patch is applied. > > We also observed that jar would occasionally raise > java.util.zip.ZipException. This was due to the fact the last two bytes > of a buffer were not apparently being copied by memcpy. Again, this > patch fixes that problem. > > The member is extremely expensive and I avoided it intentionally. > > Yes I suspected that, given how carefully that code is written. We are > aware that membars are painful on the T2 (any membar, because all > flavors are mapped to #Sync) but the risk of overlaps is not only > theoretical in this case, unfortunately :( Ok I'll install this fix after some testing and update the copy of the same code in the kernel as well, thanks.
> Several system tests of a certain (very data intensive) program crashed > as soon as it started to use multilib/niagara2_memcpy, due to memory > corruption. They run just fine after this patch is applied. > > We also observed that jar would occasionally raise > java.util.zip.ZipException. This was due to the fact the last two bytes > of a buffer were not apparently being copied by memcpy. Again, this > patch fixes that problem. > > The member is extremely expensive and I avoided it intentionally. > > Yes I suspected that, given how carefully that code is written. We are > aware that membars are painful on the T2 (any membar, because all > flavors are mapped to #Sync) but the risk of overlaps is not only > theoretical in this case, unfortunately :( Ok I'll install this fix after some testing and update the copy of the same code in the kernel as well, thanks. Thanks.
diff --git a/sysdeps/sparc/sparc64/multiarch/memcpy-niagara2.S b/sysdeps/sparc/sparc64/multiarch/memcpy-niagara2.S index b43a9e3..a1a9642 100644 --- a/sysdeps/sparc/sparc64/multiarch/memcpy-niagara2.S +++ b/sysdeps/sparc/sparc64/multiarch/memcpy-niagara2.S @@ -211,6 +211,7 @@ ENTRY(__memcpy_niagara2) */ VISEntryHalf + membar #Sync alignaddr %o1, %g0, %g0 add %o1, (64 - 1), %o4