From owner-svn-src-head@freebsd.org Thu Aug 16 13:03:45 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2D5321058BA6 for ; Thu, 16 Aug 2018 13:03:45 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A62A480C12 for ; Thu, 16 Aug 2018 13:03:44 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x243.google.com with SMTP id l7-v6so3778093iok.6 for ; Thu, 16 Aug 2018 06:03:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=wcLLm0V7WGIXY0hjU54NL/n9IW7SngCRbU57o1eyPrs=; b=nsbub+TObULlxqVoXqWQUmbjFzXg7VKnUNNApcZwvsBhex95pcJSoAZnqchZNY2VHN 6MvoFaTZTaja0NdLU1xrtE45r0QaT06q4qj/Mj5v03kiyNHWJ2RgjCRVDF+bqMi81sVf C4c8C885R9t0qromKqzp6OzGF/dAaZkF85VvAJZ77fjdWZcfayuK9Gcyb72Frq4fV3Ah OH/SVSocZDdMZxLrid6Ik9CQguJdiX9lf24atmaTXxmGHZG+QeMoTO1LtCTJ4f755XfV cjkUr+r1jnzpD8SaUQy21msi/v5LYZiZ427W1HPU5uk4KCZnnv88wiJtK8mE5ZmHUaeW zhog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=wcLLm0V7WGIXY0hjU54NL/n9IW7SngCRbU57o1eyPrs=; b=mx7aPX9wMHtoGHXTOFV8jUdrehg+n76NLfU/cDNB3FBEv/++57aiY/ECKPiRwxevdH llkGffTpp3HWmakiIbuOaYh3+IjOJckDtsoxUIou9mz+9kl0eNwkevYZQxgr3CK0TZx6 T/szlIrK9pICsdaYtl4T7G/FyhNwBmEgjwutHwkMm6bU+5mvxkM5eOe7+ljLiTDpuXxB dJuNWaDHIBPpvsjxs1lP6xFbGGL0EtpPjUcNFwx6NaMUAsiNiE2f/idKqBz9o2S2+nHv wK5/79PP+IeqoaYDDyqjQGq3IhtCR62MSxZYjCzt6gEZnhL2PnQN3EirH9frwmyeCTC0 yIlw== X-Gm-Message-State: AOUpUlHfyCXP2NNdZZpQy/LhzXm5hUmWRFr2/1NdGr7XyCW44QHAVn3p DSYrF9lt27kujy2xVyHxEcn+WGXUvwviB8gTGX6lZQ== X-Google-Smtp-Source: AA+uWPz0E0n1nxBG2rX+mR/ID9SpetZ7IA7G46Xr7psK2d3yl/1epM8/Sw5qVGirO4UuvCbbI1sS4wteX/pv8UfuB9o= X-Received: by 2002:a6b:3902:: with SMTP id g2-v6mr25792754ioa.168.1534424623290; Thu, 16 Aug 2018 06:03:43 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 2002:a4f:257:0:0:0:0:0 with HTTP; Thu, 16 Aug 2018 06:03:42 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:1052:acc7:f9de:2b6d] In-Reply-To: <18E7897D-9985-44CE-B648-2A839417B1BC@me.com> References: <201808152225.w7FMP5J2018006@repo.freebsd.org> <1534372134.1466.21.camel@freebsd.org> <05b36083-9f4d-658e-ab39-e8c317d01db7@FreeBSD.org> <18E7897D-9985-44CE-B648-2A839417B1BC@me.com> From: Warner Losh Date: Thu, 16 Aug 2018 07:03:42 -0600 X-Google-Sender-Auth: q9woTel_ek2jFMR9SoDAcZNHNKk Message-ID: Subject: Re: svn commit: r337878 - head/stand/i386/libi386 To: Toomas Soome Cc: John Baldwin , Ian Lepore , Toomas Soome , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Aug 2018 13:03:45 -0000 On Thu, Aug 16, 2018 at 1:10 AM, Toomas Soome wrote: > > > > On 16 Aug 2018, at 09:59, John Baldwin wrote: > > > > On 8/15/18 11:59 PM, Warner Losh wrote: > >> > >> > >> On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore ian@freebsd.org>> wrote: > >> > >> On Wed, 2018-08-15 at 22:25 +0000, Toomas Soome wrote: > >>> Author: tsoome > >>> Date: Wed Aug 15 22:25:05 2018 > >>> New Revision: 337878 > >>> URL: https://svnweb.freebsd.org/changeset/base/337878 < > https://svnweb.freebsd.org/changeset/base/337878> > >>> > >>> Log: > >>> libi386: remove bd_read() and bd_write() wrappers > >>> > >>> Those wroappers are nice, but do not really add much value. > >>> > >>> Modified: > >>> head/stand/i386/libi386/biosdisk.c > >>> > >>> Modified: head/stand/i386/libi386/biosdisk.c > >>> ===================================================================== > >>> ========= > >>> --- head/stand/i386/libi386/biosdisk.c Wed Aug 15 21:47:03 > >>> 2018 (r337877) > >>> +++ head/stand/i386/libi386/biosdisk.c Wed Aug 15 22:25:05 > >>> 2018 (r337878) > >>> @@ -94,10 +94,7 @@ static int nbdinfo = 0; > >>> > >>> static void bd_io_workaround(struct disk_devdesc *dev); > >>> > >>> -static int bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, > >>> - caddr_t dest); > >>> -static int bd_write(struct disk_devdesc *dev, daddr_t dblk, int > >>> blks, > >>> - caddr_t dest); > >>> +static int bd_io(struct disk_devdesc *, daddr_t, int, caddr_t, int); > >>> static int bd_int13probe(struct bdinfo *bd); > >>> > >>> static int bd_init(void); > >>> @@ -506,7 +503,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t > >>> dblk, s > >>> case F_READ: > >>> DEBUG("read %d from %lld to %p", blks, dblk, buf); > >>> > >>> - if (blks && (rc = bd_read(dev, dblk, blks, buf))) { > >>> + if (blks && (rc = bd_io(dev, dblk, blks, buf, 0))) { > >>> /* Filter out floppy controller errors */ > >>> if (BD(dev).bd_flags != BD_FLOPPY || rc != > >>> 0x20) { > >>> printf("read %d from %lld to %p, > >>> error: 0x%x\n", > >>> @@ -518,7 +515,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t > >>> dblk, s > >>> case F_WRITE : > >>> DEBUG("write %d from %lld to %p", blks, dblk, buf); > >>> > >>> - if (blks && bd_write(dev, dblk, blks, buf)) { > >>> + if (blks && bd_io(dev, dblk, blks, buf, 1)) { > >>> DEBUG("write error"); > >>> return (EIO); > >>> } > >>> @@ -713,20 +710,6 @@ bd_io(struct disk_devdesc *dev, daddr_t dblk, > >>> int blks > >>> } > >>> > >>> return (0); > >>> -} > >>> - > >>> -static int > >>> -bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t > >>> dest) > >>> -{ > >>> - > >>> - return (bd_io(dev, dblk, blks, dest, 0)); > >>> -} > >>> - > >>> -static int > >>> -bd_write(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t > >>> dest) > >>> -{ > >>> - > >>> - return (bd_io(dev, dblk, blks, dest, 1)); > >>> } > >>> > >>> /* > >>> > >> > >> This would be a more satisfying change if there were something like > >> > >> #define BD_RD 0 > >> #define BD_WR 1 > >> > >> so that it was clear at a glance what a bd_io() call is doing. > >> > >> > >> I think that's a good idea... > > > > Arguably the bd_read/write wrappers were even clearer (and there purpose > > was readability in that case). > > Yes thats true, but also will leave us in mercy of inlining etc.. anyhow, > *my* purpose is to get the line of changes done (to be able to perform IO > with >512 sector size, merge with bioscd.c and split up floppy, cd and hdd > cases, so the user can distinguish the devices. > OK, Now I'm confused. All this sounds like regression, apart from the >512 sector thing. Warner