From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 21A52C00144 for ; Tue, 2 Aug 2022 00:03:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 8F47B83AD1; Tue, 2 Aug 2022 02:03:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="DQbHjytN"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5805783F82; Tue, 2 Aug 2022 02:03:08 +0200 (CEST) Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 222278300F for ; Tue, 2 Aug 2022 02:03:05 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pj1-x102e.google.com with SMTP id w17-20020a17090a8a1100b001f326c73df6so12080400pjn.3 for ; Mon, 01 Aug 2022 17:03:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc; bh=9uObrM6GUAKNq+TCBLBeK3bs7uzUOsR5wes6ezB+pW0=; b=DQbHjytNEqJpxpzgqdV3/QKc/gbRQagCQAS2p7bpJQ/tuXKsWTagrN+MuihSSC9aDO FNjtNaSg74h/QJMmaX26QTTihYGZJwdgdgNJRInVKos00OD5i/WBJ3TCARE8I1tIciwl bp3Q5QNofGTBnwzEbpyoddmM+6Upixc9hZhv1E68iezVKZPMH9ia0LZbPDicphm0RBqN IXVKxeeJGwUKOks1lu59STjnUh8l3zy81ATz8vaLNnh7DiEbu/1k64drNfMCH3kHtEaQ UuJ+2VUdyv2TFqiNc0rZuerd1wfeiDFkfyl3PrHpBxq4LsWdSpwRLP/o5MOUQEtAgJu0 ukwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc; bh=9uObrM6GUAKNq+TCBLBeK3bs7uzUOsR5wes6ezB+pW0=; b=wxUIcH5EE5VuoAaQaB7CdJmmj3t5T6qakdOINM8stryxkpXskU4oHmsnBKHR9tuZV8 mjQadilHV8xgjZVnWGYfg3HuIRNkQYeePiICORl6BMaF3ksacHvV/2VDvur6y2D61fF4 06KpA2jlzeYHsUmBEmNyzgY29hOOsOfqP6cN1zKDv7Wp0zSLWN02OvWxqe4RPztledTA GMwYiUnkj1whJL9EnkFQrSWkcIFxIsMnPVOlHWInJoZwFmhKTrW4suwsxikttB/QJwsR Q2P9Ykxto5vOEdC7q402AORH4wzg6VbkC93RBB8DI16WEY4gj/WpFpmckBmVQ/QacKey EiXQ== X-Gm-Message-State: ACgBeo3aD9HeBfoVb8EEilhHouUXrxT//EfXDKOcI3CRvuky2+aI6ysX 8/mE0z9dUu+YXES8bUWpFTGlQQ== X-Google-Smtp-Source: AA6agR6/7dpSnG6v1JDHveM8/WpfHI8PeLLeWd1j4RWB9ke2l44eea5gDWEtQmVBfV+VsrQU5ss9nQ== X-Received: by 2002:a17:90a:68ce:b0:1f4:db83:1eea with SMTP id q14-20020a17090a68ce00b001f4db831eeamr14120636pjj.55.1659398583257; Mon, 01 Aug 2022 17:03:03 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:4407:5475:fc77:828c]) by smtp.gmail.com with ESMTPSA id g6-20020a625206000000b0052d1b19b7f1sm4075647pfb.204.2022.08.01.17.02.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 Aug 2022 17:03:02 -0700 (PDT) Date: Tue, 2 Aug 2022 09:02:57 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: Tom Rini , u-boot@lists.denx.de Subject: Re: [PATCH v2 1/5] fs: fat: finding an empty FAT cluster Message-ID: <20220802000257.GA53591@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , Tom Rini , u-boot@lists.denx.de References: <20220731115837.77646-1-heinrich.schuchardt@canonical.com> <20220731115837.77646-2-heinrich.schuchardt@canonical.com> <20220801010244.GA37247@laputa> <21dd9f41-0f2b-c88b-b9e3-d89dd92ee952@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <21dd9f41-0f2b-c88b-b9e3-d89dd92ee952@canonical.com> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Aug 01, 2022 at 10:21:20AM +0200, Heinrich Schuchardt wrote: > > > On 8/1/22 03:02, AKASHI Takahiro wrote: > > On Sun, Jul 31, 2022 at 01:58:33PM +0200, Heinrich Schuchardt wrote: > > > Currently we have two functions with redundant coding to find an empty > > > cluster: > > > > > > * find_empty_cluster() seeks from the beginning of the FAT table > > > * determine_fatent() seeks after a given entry > > > > > > Both do not detect the end of the FAT table correctly and return an invalid > > > cluster number if no empty entry if found. > > > > > > find_empty_cluster() is replaced by an invocation of determine_fatent(). > > > > > > determine_fatent() is changed to seek in a second round from the beginning > > > of the FAT table and to return an error code if no free entry is found. > > > With this patch we will always find an empty cluster if it exists. > > > > > > Further patches are needed to handle the disk full error gracefully. > > > > > > Signed-off-by: Heinrich Schuchardt > > > --- > > > v2: > > > no change > > > > I made this comment before: > > https://lists.denx.de/pipermail/u-boot/2022-July/488827.html > > > > -Takahiro Akashi > > The speedup only exists in the rare case that the disk is full. > Therefore reducing the code size and complexity has priority. I don't believe that my approach is complexed at all. -Takahiro Akashi > Best regards > > Heinrich > > > > > > --- > > > fs/fat/fat_write.c | 56 ++++++++++++++++++++++++---------------------- > > > 1 file changed, 29 insertions(+), 27 deletions(-) > > > > > > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > > > index 8ff2f6def0..a137e14f41 100644 > > > --- a/fs/fat/fat_write.c > > > +++ b/fs/fat/fat_write.c > > > @@ -536,22 +536,41 @@ static int set_fatent_value(fsdata *mydata, __u32 entry, __u32 entry_value) > > > return 0; > > > } > > > -/* > > > - * Determine the next free cluster after 'entry' in a FAT (12/16/32) table > > > - * and link it to 'entry'. EOC marker is not set on returned entry. > > > +/** > > > + * determine_fatent() - get next free FAT cluster > > > + * > > > + * The parameter @entry indicates the current cluster. To reduce fragementation > > > + * the function first searches for a free cluster after the current cluster. > > > + * If none is found, the search is repeated from the beginning of the FAT table. > > > + * > > > + * If @entry is set, the new FAT entry is appended to the given one. > > > + * If @entry is zero, only the number of the first free cluster is returned. > > > + * > > > + * @entry: current entry > > > + * Return: next free cluster or negative error > > > */ > > > -static __u32 determine_fatent(fsdata *mydata, __u32 entry) > > > +static int determine_fatent(fsdata *mydata, __u32 entry) > > > { > > > - __u32 next_fat, next_entry = entry + 1; > > > + __u32 next_fat, next_entry = entry; > > > + int second_round = 0; > > > while (1) { > > > + ++next_entry; > > > + if (CHECK_CLUST(next_entry, mydata->fatsize)) { > > > + if (!second_round) { > > > + second_round = 1; > > > + next_entry = 3; > > > + } else { > > > + return -ENOSPC; > > > + } > > > + } > > > next_fat = get_fatent(mydata, next_entry); > > > - if (next_fat == 0) { > > > + if (!next_fat) { > > > /* found free entry, link to entry */ > > > - set_fatent_value(mydata, entry, next_entry); > > > + if (entry) > > > + set_fatent_value(mydata, entry, next_entry); > > > break; > > > } > > > - next_entry++; > > > } > > > debug("FAT%d: entry: %08x, entry_value: %04x\n", > > > mydata->fatsize, entry, next_entry); > > > @@ -794,23 +813,6 @@ get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer, > > > return 0; > > > } > > > -/* > > > - * Find the first empty cluster > > > - */ > > > -static int find_empty_cluster(fsdata *mydata) > > > -{ > > > - __u32 fat_val, entry = 3; > > > - > > > - while (1) { > > > - fat_val = get_fatent(mydata, entry); > > > - if (fat_val == 0) > > > - break; > > > - entry++; > > > - } > > > - > > > - return entry; > > > -} > > > - > > > /** > > > * new_dir_table() - allocate a cluster for additional directory entries > > > * > > > @@ -824,7 +826,7 @@ static int new_dir_table(fat_itr *itr) > > > int dir_oldclust = itr->clust; > > > unsigned int bytesperclust = mydata->clust_size * mydata->sect_size; > > > - dir_newclust = find_empty_cluster(mydata); > > > + dir_newclust = determine_fatent(mydata, 0); > > > /* > > > * Flush before updating FAT to ensure valid directory structure > > > @@ -1066,7 +1068,7 @@ set_clusters: > > > /* Assure that curclust is valid */ > > > if (!curclust) { > > > - curclust = find_empty_cluster(mydata); > > > + curclust = determine_fatent(mydata, 0); > > > set_start_cluster(mydata, dentptr, curclust); > > > } else { > > > newclust = get_fatent(mydata, curclust); > > > -- > > > 2.36.1 > > >