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 65F5DC00140 for ; Mon, 1 Aug 2022 01:02:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7081B80741; Mon, 1 Aug 2022 03:02:55 +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="d4sGNrb1"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id EE1CA8020D; Mon, 1 Aug 2022 03:02:53 +0200 (CEST) Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) (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 B7D7F839C3 for ; Mon, 1 Aug 2022 03:02:50 +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-x1031.google.com with SMTP id h21-20020a17090aa89500b001f31a61b91dso10713244pjq.4 for ; Sun, 31 Jul 2022 18:02:50 -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=cWjwASqCMosn+YxNJuaVQCx8NcRgAY6pRzZ7MDLVt+o=; b=d4sGNrb1+4dIwVc8ZLaTUV9zpWYEMCfd/pq9DE0aqlRk2xZaJLVRyLafMSVtx4y46a aRdAx+oTVUCgTDfMrVaiRd18bhOyTYKxHLJeJTZ62TRgQhsdZal3RXjOQk64w98XtaAB cRQpsRVx58SUIxVcXiP79D1eg/WbKjeemLQ+ThLuJb88xXgkafeRk/K1IMRfQLveQoAC QbHgAvd6Qz688iGAGDulLKHrTsmtmO0HSFRd1kYgwbGiA/ZzMdYEHH8fe4A9oFsf0WBT OsBhd8M67rkZa8ShlMJkmlZs4CKl94tVY5cWtpX1BwuYqWZ3pQp586RlewtTRakfX0+F yqqw== 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=cWjwASqCMosn+YxNJuaVQCx8NcRgAY6pRzZ7MDLVt+o=; b=Finv+asvGlX+e8hhcWz8r/9TlQcpJT2J/fI3xGpjeV9giZ35s1s3u6jEG3ieexTKG3 UMyYW5DRIseGsYS8vHS0TvHMzvuxagem0btI1L1JdeAtcLBWoAsyq0spbqFRUrebVt3a ItoLPXy9TVk04Luz26WMDCaeS0bcQR7U0dIvYmz9RVR1CdJUBKAaDjGQOkzPKwU0S94l /Lg3j4QxeZJWhrCVEZc4A4hCIMUzPxI8UTXRIEjG/h1FU/x76rkulPmhMTHBXXMsK2PO RFjOjTnhB0q7gQq14iWIpTpGPJu8pyLUTuoAb0FoMSXqezPYAAZXuGkxmE1f4Z6lMNNQ 4xAg== X-Gm-Message-State: ACgBeo2u0VCRmW8flzcCQISiTp3JhihGfEGX2kvDRs6gQ0xV9ksoYb0f 9SChpXpXMNlCm/DxAq3gL26slQ== X-Google-Smtp-Source: AA6agR490EgEs29W9k2BYbcMQKkZz4srFn9tv5qRH0XRoVg+HwRNBRHeGOe37E26JKnyjuoev6MAFQ== X-Received: by 2002:a17:90a:6007:b0:1f3:3527:609e with SMTP id y7-20020a17090a600700b001f33527609emr15531874pji.52.1659315768494; Sun, 31 Jul 2022 18:02:48 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:6d35:92a:3690:8d3a]) by smtp.gmail.com with ESMTPSA id d8-20020a17090a7bc800b001f2ef3c7956sm10137873pjl.25.2022.07.31.18.02.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 31 Jul 2022 18:02:47 -0700 (PDT) Date: Mon, 1 Aug 2022 10:02:44 +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: <20220801010244.GA37247@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220731115837.77646-2-heinrich.schuchardt@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 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 > --- > 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 >